ADR 0034 documentation standard publication - #842
Conversation
| - Follow the MADR structure from the template on the ADR index (`docs/architecture/adr/index.mdx`): | ||
| frontmatter (`adr`, `status`, `date`, `tags`), Context and problem statement, Considered options, | ||
| Decision outcome |
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the Code Review DetailsNo findings. Verification performed: every relative link and reference definition in the new documentation |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| audience callout necessary. Every isolated (not directly attached to code) document MUST include an | ||
| audience the document is written for. |
There was a problem hiding this comment.
❓ QUESTION: Does rule 3's audience requirement apply to ADRs?
Context
The ADR template added in this PR (docs/architecture/adr/index.mdx, Format section) has no audience callout, and neither does ADR-0034. ADRs are isolated documents, so as written every ADR on the site is a rule 3 violation — and "Changes to this standard" says existing violations are named, never silently grandfathered.
Either the ADR template carries an audience line, or this section states that ADRs are exempt because the audience is implicit.
withinfocus
left a comment
withinfocus
left a comment
There was a problem hiding this comment.
Where's the CF Pages preview?
| - Follow the MADR structure from the template on the ADR index (`docs/architecture/adr/index.mdx`): | ||
| frontmatter (`adr`, `status`, `date`, `tags`), Context and problem statement, Considered options, | ||
| Decision outcome |
Audit showed that clients was the one that needed attention, but drift may occur between proposal and acceptance. We need to bring all repos up to compliance
d6939b4 to
a98cbc3
Compare
Deploying contributing-docs with
|
| Latest commit: |
665bd2a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0164273d.contributing-docs.pages.dev |
| Branch Preview URL: | https://arch-docs-documentation-stan.contributing-docs.pages.dev |
withinfocus
left a comment
withinfocus
left a comment
There was a problem hiding this comment.
This looks good to me but will wait until the ADR is approved.
Claude Configuration Review — PR #842Verdict: ✅ Approve — no critical or major issues found. Scope reviewed: What changedThe PR makes a single, four-line edit to - ADRs go in `docs/architecture/adr/` with format `####-title.md`
-- Follow existing ADR structure (Status, Context, Decision, Consequences)
+- Follow the MADR structure from the template on the ADR index (`docs/architecture/adr/index.mdx`):
+ frontmatter (`adr`, `status`, `date`, `tags`), Context and problem statement, Considered options,
+ Decision outcomeThis keeps the repo's Claude instructions in sync with the MADR standard that the rest of the PR
Every claim in the added text is accurate. The old text it replaces ( 1. Plugin validation — skipped (not applicable)No plugin directories changed and this repository contains no 2. Skill review — skipped (not applicable)No 3. Configuration & security review — run (
|
| Check | Result |
|---|---|
settings.local.json committed to git |
Not tracked ✅ |
Hardcoded secrets / API keys / tokens / passwords in .claude/ |
None found (regex sweep for api_key, secret, password, token, bearer, sk-*, ghp_*, AKIA*) ✅ |
| Dangerous command auto-approvals | .claude/settings.json declares no permissions block — nothing auto-approved ✅ |
| Overly broad file access permissions | None declared ✅ |
| Sensitive paths exposed in instructions | None ✅ |
Tracked files under .claude/ |
Only CLAUDE.md and settings.json ✅ |
.claude/settings.json contains only attribution (empty commit/PR attribution strings) and
extraKnownMarketplaces pointing at the first-party bitwarden/ai-plugins GitHub repo. It uses the
github source type rather than an arbitrary URL, and is unchanged by this PR.
Structure & reference integrity — ✅ clean
All twelve paths referenced by CLAUDE.md resolve on disk, including the newly added
docs/architecture/adr/index.mdx. No broken file references.
The file is 122 lines / ~570 words — well within a healthy size for a project CLAUDE.md, with clear
section headers, core directives stated upfront, and detail delegated to referenced docs rather than
duplicated.
Findings
No errors (must-fix). Two minor observations (should-fix / optional), neither blocking:
Minor — .claude/CLAUDE.md:32 — invalid npm invocation (pre-existing, not touched by this PR)
npm start:insecure # Start without SSLstart:insecure is not an npm lifecycle script, so this command fails as written. It should be:
npm run start:insecure # Start without SSLConfirmed against package.json, which defines start:insecure as a regular (non-lifecycle) script.
npm start on the line above is correct, since start is a lifecycle script. This predates the PR
— worth a follow-up commit rather than a blocker here.
Suggested — .claude/CLAUDE.md:111-113 — inline structure summary can drift from the template
The new bullet both points at the authoritative template and restates its frontmatter fields and
section names. That inline summary is genuinely useful for one-shot context, but it is the same class
of duplication that just caused the stale Status, Context, Decision, Consequences text this PR is
fixing. Two equally reasonable options:
- Keep as-is (recommended) — the summary is short, currently accurate, and saves Claude a file
read; the template link makes the source of truth unambiguous. - Trim to the pointer —
Follow the MADR template on the ADR index (docs/architecture/adr/index.mdx)— eliminates drift risk entirely at the cost of an extra read.
For completeness, the summary omits two template elements (the # NNNN - Short imperative title
heading and the <AdrTable frontMatter={frontMatter}></AdrTable> component at index.mdx:100-102).
Since the bullet directs readers to the template, this is not a defect.
Environment note
The checkout has an uncommitted working-tree modification that reverts this PR's CLAUDE.md
change (git status shows M .claude/CLAUDE.md; the on-disk file still reads
Follow existing ADR structure (Status, Context, Decision, Consequences)). This appears to be an
artifact of the review environment, not part of the PR. This review evaluated the committed PR
head as reported by gh pr diff 842, which contains the MADR text. No action needed on the PR;
flagging only so the discrepancy is not mistaken for a finding.
Summary
| Severity | Count |
|---|---|
| Critical | 0 |
| Major | 0 |
| Minor / Suggested | 2 (one pre-existing, one optional) |
The CLAUDE.md change is accurate, verified line-by-line against docs/architecture/adr/index.mdx,
correctly resolves drift between the Claude instructions and the ADR template, and introduces no
security or structural issues. Safe to merge.
withinfocus
left a comment
withinfocus
left a comment
There was a problem hiding this comment.
Looks great, but holding until Council occurs.
3 participants
📔 Objective
Approve and publish unified documentation standard.
Related PRs: