Skip to content

docs(adr): record bridge + logging decisions from the 0.8.1 fixes (ADR-0005–0008)#62

Open
stephane-segning wants to merge 1 commit into
mainfrom
claude/adrs-session-decisions
Open

docs(adr): record bridge + logging decisions from the 0.8.1 fixes (ADR-0005–0008)#62
stephane-segning wants to merge 1 commit into
mainfrom
claude/adrs-session-decisions

Conversation

@stephane-segning

Copy link
Copy Markdown
Contributor

1. Summary

Adds four Architecture Decision Records capturing the load-bearing, non-obvious decisions behind the 0.8.1 browser-bridge and logging fixes, so the why survives without reverse-engineering it from the diffs. No code change — docs only.

  • ADR-0005 — atomic on-disk state (per-writer pid+uuid temp + rename), unifying the oauth2/models-info caches and bridge.json.
  • ADR-0006bridge.json as the bridge-token source of truth: host-only write, reload-on-mismatch, explicit-token authority.
  • ADR-0007 — handshake rejection: explicit rejected frame, slow-retry not dormant (records the reversal + the live incident behind it), non-secret fingerprint logging.
  • ADR-0008 — a trace log tier gated by OpenCode's DEBUG.

Source of truth: the merged PRs whose decisions these record — #54, #55, #56, #57 (and the existing #47 / ADR-0001 set the format precedent).

2. Intent

Close the documentation gap: those four fixes shipped with symptom-keyed troubleshooting + CHANGELOG entries, but the decisions (and the alternatives we rejected — e.g. file-locking vs rename-atomicity, dormant vs slow-retry, a separate log knob vs reusing DEBUG) weren't recorded as ADRs. The repo's ADR bar ("a choice that closes off alternatives someone would reasonably reach for") fits all four.

3. Scope

In Scope

  • Four new ADRs (docs/adr/00050008), numbered after the existing code-index ADRs (0002–0004), with inter-ADR cross-references wired.
  • ADR index table (docs/adr/README.md), CHANGELOG Documentation entry, and cross-links from browser.md / architecture.md / troubleshooting.md and the CLAUDE.md/AGENTS.md ADR pointer (kept in sync).

Out of Scope

  • No source/behavior change. No new RFCs (RFCs are for proposed work; these decisions already shipped). Package READMEs already exist and are current.

4. Verification

  • Running automated tests — n/a (docs only); pnpm lint + pnpm format:check pass.
  • Checking logs — n/a
  • Verified: every relative ADR link resolves to a file on disk; CLAUDE.md and AGENTS.md ADR line identical; no duplicate ADR numbers; each ADR cites real file paths/events verified against the current code (writeBridgeFile, adoptRotatedToken, tokenFingerprint, REJECTED_RETRY_MS, fromOpenCodeLogLevel).
format:check=0   lint=0
ADR link targets: 0001–0008 all ✓ exist
CLAUDE.md == AGENTS.md ADR line: IN SYNC

5. Screenshots / Evidence

ADR index now lists 0001–0008; the four new records mirror ADR-0001's MADR-ish shape (Status/Scope, Context, Decision, Consequences split Positive/Negative, ### … — rejected alternatives). ADR-0007 records the dormant→slow-retry reversal with the field incident as the rationale.

6. Risk Assessment

Negligible — documentation only, no runtime/build/test surface touched. Worst case is a doc inaccuracy; facts were cross-checked against current source and the merged PRs.

7. AI Usage Declaration

AI (Claude Code) drafted the four ADRs from the merged PRs' decisions — four subagents, one ADR each, briefed with the specific decision + the ADR-0001 format and told to verify claims against the actual code. The human author renumbered/reconciled them, wired cross-links, and reviewed for accuracy and voice.

  • I reviewed every ADR for technical accuracy against the code and the PRs.
  • I am accountable for the content; the alternatives and consequences reflect real decisions, not invented ones.

8. Reviewer Focus

The Alternatives considered sections — that's the load-bearing part. In particular ADR-0007's dormant-vs-slow-retry reversal and ADR-0006's "host adopts the file token" trust-surface note. Confirm the cited mechanisms still match main.

🤖 Generated with Claude Code

… (ADR-0005–0008)

Capture the load-bearing, non-obvious decisions behind the #54#57 work as
Architecture Decision Records, matching ADR-0001's MADR-ish shape
(Context / Decision / Consequences / Alternatives-with-why):

- ADR-0005 — atomic on-disk state (per-writer pid+uuid temp + rename), unifying
  the oauth2/models-info caches (#54) and bridge.json (#57).
- ADR-0006 — bridge.json as the token source of truth: host-only write,
  reload-on-mismatch, explicit-token authority (#57).
- ADR-0007 — handshake rejection: explicit `rejected` frame, slow-retry (not
  dormant — records the reversal + the incident behind it), fingerprint
  logging (#55).
- ADR-0008 — a `trace` log tier gated by OpenCode's DEBUG (#56).

Renumbered to 0005–0008 (0002–0004 are the existing code-index ADRs), wired the
inter-ADR cross-references and the index table, and cross-linked them from
browser.md / architecture.md / troubleshooting.md and the CLAUDE.md/AGENTS.md
ADR pointer. CHANGELOG Documentation entry added. Docs-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

✅ AI Governance check passed

This PR declares AI usage, references a source of truth, and provides verification evidence. Thank you.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces four new Architecture Decision Records (ADRs 0005 through 0008) documenting core architectural decisions around atomic on-disk writes, bridge token management, handshake rejection handling, and the introduction of a trace log tier. It also updates several documentation files (including CHANGELOG.md, AGENTS.md, and CLAUDE.md) to cross-reference these new records. The reviewer provided constructive feedback to ensure consistency in the ADR index table of contents by adding a missing suffix to the title of ADR-0007.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread docs/adr/README.md
| [0004](0004-code-index-tree-sitter-sound-but-partial-resolution.md) | Code index call graph: tree-sitter only, "sound but partial" | Accepted (experimental) |
| [0005](0005-atomic-file-writes-per-writer-temp.md) | Atomic on-disk state: per-writer temp file + rename | Accepted |
| [0006](0006-bridge-token-source-of-truth.md) | Bridge token: `bridge.json` as single source of truth (host-only write, reload-on-mismatch) | Accepted |
| [0007](0007-bridge-handshake-rejection.md) | Bridge handshake rejection: explicit reject frame, slow-retry, fingerprint logging | Accepted |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The title of ADR-0007 in the README table of contents is missing the (not dormant) suffix. Let's update it to be fully consistent with the actual title of the ADR file (0007-bridge-handshake-rejection.md).

Suggested change
| [0007](0007-bridge-handshake-rejection.md) | Bridge handshake rejection: explicit reject frame, slow-retry, fingerprint logging | Accepted |
| [0007](0007-bridge-handshake-rejection.md) | Bridge handshake rejection: explicit reject frame, slow-retry (not dormant), fingerprint logging | Accepted |

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0deded6f9f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +4 to +5
- **Scope:** `@vymalo/opencode-browser` and `@vymalo/opencode-browser-mcp` — the bridge token
lifecycle and its per-user `bridge.json` state file.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply bridge-token decision to browser-mcp

Because this ADR's scope includes opencode-browser-mcp, the decision as written is inaccurate: packages/opencode-browser-mcp/src/mcp.ts still calls writeBridgeFile(port, token) immediately after resolveSharedToken and then calls createEndpoint without onHost or reloadToken. In an MCP process that loses the bind and becomes a guest, it can still write/rotate bridge.json, and an MCP host won't adopt a rotated token, so the documented host-only/reload guarantee doesn't hold for the MCP server.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant