docs(adr): ADR-0099 operator OpenCode config overlay for review (file-based base + full override) - #460
Conversation
…-based base + full override)
Records the owner-directed decision (2026-07-17) to (1) move the review OpenCode config
from a Rust json! blob to a checked-in OpenCode-native review.jsonc, and (2) let a trusted
SysAdmin supply a `review.opencode` overlay deep-merged host-side with FULL OVERRIDE, so
they can add custom sub-agents/models/providers and different per-agent access models.
Documents the three-layer model (base file → runtime injection → operator overlay), the
exact keys the runtime injects/overrides internally (SysAdmin-facing contract), the
coverage/read-only floor WARNING (awareness over prevention, per the owner), and the
constraints (opencode rejects unknown keys; secrets stay {env:*}; the untrusted checkout is
still never a config source, reaffirming ADR-0097 #6). Complements the mediated ADR-0066 MCP
path as the trusted-operator power tool.
Implementation is a follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ AI Governance check passedThis PR declares AI usage, references a source of truth, and provides verification evidence. Thank you. |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces ADR-0098, which outlines the design for operator-supplied OpenCode config overlays for review, transitioning the review configuration from a Rust blob to a checked-in file-based base with full override capabilities. It also updates the ADR index in docs/adr/README.md. The review feedback requests changing instances of future tense (such as "it will be replaced" and "won't start") to declarative present tense throughout the new ADR and its index entry, in accordance with the standard style for Architecture Decision Records.
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.
| | key | who sets it | can the overlay change it? | | ||
| |---|---|---| | ||
| | `model`, `provider.eaig.*` | base `{env:*}` from the ai-models chart | yes (point a tier elsewhere) | | ||
| | `agent.review.prompt` | runtime `{file:*}` (per-task, dynamic) | yes, but it will be replaced each task unless the overlay sets a different agent | |
There was a problem hiding this comment.
According to the general rules, Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense, even if the features are not yet implemented in the codebase. Please use present tense instead of future tense ("it will be replaced").
References
- Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense, even if the features are not yet implemented in the codebase. Do not change the tense to future tense for planned or proposed implementation details in ADRs.
|
|
||
| - **OpenCode's schema rejects unknown keys** (verified 1.18.2: even a `"//"` string-key fails with | ||
| *"Unrecognized key"*). A typo'd or non-schema key in the overlay fails the **entire** config and the | ||
| review won't start — the overlay must be valid OpenCode config. |
There was a problem hiding this comment.
According to the general rules, Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense. Please use present tense instead of future tense ("won't start").
| review won't start — the overlay must be valid OpenCode config. | |
| review does not start — the overlay must be valid OpenCode config. |
References
- Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense, even if the features are not yet implemented in the codebase. Do not change the tense to future tense for planned or proposed implementation details in ADRs.
| read-only floor. Mitigated by the render-time WARNING + coverage disclosure, not prevented — this is | ||
| the accepted cost of the owner's full-override choice. | ||
| - **A malformed overlay fails the whole review** (opencode's strict schema). The failure is loud | ||
| (review won't start) rather than silent, but it is an operational footgun; the docs call it out. |
There was a problem hiding this comment.
According to the general rules, Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense. Please use present tense instead of future tense ("won't start").
| (review won't start) rather than silent, but it is an operational footgun; the docs call it out. | |
| (review does not start) rather than silent, but it is an operational footgun; the docs call it out. |
References
- Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense, even if the features are not yet implemented in the codebase. Do not change the tense to future tense for planned or proposed implementation details in ADRs.
| | [0095](0095-opencode-plugins-recording-and-gates.md) | Recording + gate enforcement live in first-party OpenCode plugins (`integrations/opencode/plugins/*`): a **recorder** (in-process `tool.execute.before/after` + reasoning parts off the event bus → right-bytes JSONL → ADR-0034 transcript store; captures subagent-internal calls an ACP client can't see) and a **gate-interlock** (per-session state; `tool.execute.before` **throws** on the terminal tool until refute/coverage preconditions hold — block-until enforcement on the *stable* hook, since `chat.params` can't express `force_names()`-style force-now; `experimental.*` hooks carry steering only, never enforcement); OpenCode version-pinned, every bump re-runs the RFC-0009 probe | Proposed | | ||
| | [0096](0096-mediated-forge-read-tools.md) | Forge **read** tools on the mediated MCP surface — the App key stays control-plane-side. The agent (open/review, incl. the ADR-0094 OpenCode host + its read-only `explore` subagent) reads other branches/issues/PRs/files-at-ref via curated read-only tools on the existing `lightbridge` MCP endpoint it already calls (task-scoped runner token); the control plane performs the forge call with a read-only, **task-repo-scoped**, ≤1h installation token it mints+refreshes **server-side**. Pod gains the capability with **zero new credential and zero new egress** (never holds a token, never reaches api.github.com) and no token-expiry failure on long tasks. Curated on the ADR-0072 `CodePlatform` client (multi-forge, tight surface) rather than adopting github-mcp-server in-tree (kept as `--read-only` fallback). Extends ADR-0037/0020 from writes to reads. Rejects: scoped-token-in-pod (widens ADR-0088 egress+creds; token expiry) and App-key-in-pod (master credential in the untrusted-code sandbox — hard no). | Proposed | | ||
| | [0097](0097-review-runs-on-opencode.md) | **Review runs on OpenCode** ([RFC-0009](../rfc/0009-opencode-acp-agent-host.md) Phase 3 — reverses its Phase-2/3 phasing): a thin transport host over a **reused Rust core**. OpenCode owns its agent loop; one `session/prompt` runs a whole cycle. Native policies split — budgets/wind-down/batching (ADR-0042) → OpenCode's loop; the **coverage + refute gates (ADR-0041/0091) → supervisor-side, reusing the exact `TurnPolicy` code** (no TS reimplementation). Load-bearing: the gate input is the recorder JSONL not ACP updates (sees subagent-internal reads); built-in tools disabled at the config **TOP LEVEL** not per-agent (ACP runs the default `build` agent, so a per-agent block is ignored → the model would get built-in `read`/`grep` and escape coverage); stdio review MCP via the config `mcp` block; the cutover gate is a **shadow parity run** (`xtask shadow diff`), not the #252 eval harness. Host built + proven vs real OpenCode 1.18.3 (#442–#449); shadow gate #450; **live dispatch cutover (slice 5) pending, shadow-gated**. Does not yet supersede ADR-0026 (native review) until the cut. | Accepted (host built/proven; live cutover slice-5 shadow-gated) | | ||
| | [0098](0098-operator-opencode-config-overlay.md) | **Operator-supplied OpenCode config overlay for review** (builds on ADR-0097, reaffirms its #6 checkout-isolation): move the review config from a Rust `json!` blob to a checked-in OpenCode-native `review.jsonc` (like `open` mode), and let a **trusted SysAdmin** (ai-helm-values, NOT the untrusted checkout) supply a `review.opencode` overlay that is **deep-merged host-side with FULL OVERRIDE** (overlay wins on every key). Three documented layers: base file → runtime injection (per-task prompt via `{file:*}`, attribution headers, tier `reasoning`, temperature) → operator overlay. Full override can weaken the coverage/read-only floor (custom sub-agents with different access models); the system **warns + coverage-discloses** when the floor is breached rather than forbidding it (owner's choice: awareness over prevention). Constraints documented for the SysAdmin: opencode's schema **rejects unknown keys** (a typo fails the whole config → review won't start); secrets stay `{env:*}`; checkout is still never a config source. Complements the mediated ADR-0066 MCP path (safe default) as the power-tool for trusted operators. | Accepted (design; implementation follow-up) | |
There was a problem hiding this comment.
According to the general rules, Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense. Please use present tense instead of future tense ("won't start").
References
- Architecture Decision Records (ADRs) should describe the decided target design in declarative present tense, even if the features are not yet implemented in the codebase. Do not change the tense to future tense for planned or proposed implementation details in ADRs.
![lightbridge-assistant[bot]](https://avatars.githubusercontent.com/u/128943481?s=60&v=4){kind=small}
There was a problem hiding this comment.
🅵 Fast automated pass — SAST + a quick, diff-scoped look (no repo-wide retrieval). For a deeper, repo-aware review, mention @lightbridge-assistant on this PR.
Reviewed ADR-0098: Approved pending clarification in section 4.1 about the implementation location for logging warnings when the coverage/read-only floor is breached. The design is sound, properly references DISABLED_BUILTINS and ADR-0097, and correctly describes the three-layer config model (base + runtime injection + operator overlay) with acceptable risk mitigation (warn + disclosure). A minor documentation improvement to specify WHERE the warning should be logged would help future implementers, but this does not block acceptance. Ready for implementation follow-up.
🤖 AI-generated review — treat it as untrusted, verify before acting; a human owns the final decision (AI governance).
| oc["opencode acp<br/>(neutral cwd, empty HOME/XDG)"] | ||
|
|
||
| base --> merge | ||
| inject --> merge |
There was a problem hiding this comment.
Implement-floor-warning-location-not-modelled
Section 4.1 states: "at render time the supervisor diffs the merged config against the base floor and logs a WARNING naming each invariant the overlay relaxed". This is an implementation detail that isn't captured in the design model. Suggest either specifying where this warning should be emitted (supervisor vs control plane) or marking it as TBD to clarify that this is for implementation to decide. This would eliminate ambiguity for future implementers.
Was this useful? React 👍/👎 to give us feedback
|
@lightbridge-assistant review |
There was a problem hiding this comment.
Lightbridge review
ADR-0098 is well-structured, internally consistent, and properly cross-referenced to ADR-0097 (checkout isolation) and ADR-0066 (mediated MCP). The three-layer merge model (base → runtime → operator overlay), the full-override-with-warning design, and the security boundary (trusted operator config ≠ untrusted checkout) are all clearly articulated. The prior P2 finding about warning-location ambiguity is retracted — section 4 already specifies "the supervisor" logs at render time, which is adequate for an ADR. No issues found.
ℹ️ 2 additional changed file(s) carry low review signal and aren't counted above: 2 config/docs.
🤖 AI-generated review — treat it as untrusted, verify before acting; a human owns the final decision (AI governance).
…en by A2A #458) + present-tense fixes PR #458 (feat(a2a): stream review findings per-finding) already owns docs/adr/0098-a2a-per-finding-review-streaming.md, so this ADR takes the next free number, 0099. Also addresses gemini's ADR-tense nits: convert the two genuinely future-tense statements ("will be replaced", "won't start") to present tense. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
stephane-segning
changed the title
|
Addressed the review + one thing the bots didn't catch: Renumbered ADR-0098 → ADR-0099. PR #458 ( @gemini-code-assist — tense nits: fixed the two genuinely future-tense statements — the @lightbridge-assistant — the P2 "floor-warning-location-not-modelled": agreed with your own deeper pass, which already retracted it — §4 does specify "the supervisor … at render time," so the location isn't ambiguous and there's nothing to change. Meta-note for calibration: the fast pass flagged as a P2 something that was answered two sentences down in the same section; the deep pass caught that and self-retracted, which is the right outcome — the signal is that the fast pass anchored on "implementation detail" without checking whether §4 already resolved it. |
|
Thank you for the update and for addressing the tense nits. The renumbering to ADR-0099 and the clarifications regarding the design intent and the handling of the floor-warning location are well-noted. Your explanation of the distinction between the fast-pass signal and the deep-pass resolution is clear and aligns with the expected behavior of the system. This ADR provides a solid foundation for operator-supplied OpenCode config overlays. |
1 participant
1. Summary
This PR changes:
json!blob to a checked-in OpenCode-nativereview.jsonc, and (b) let a trusted SysAdmin supply areview.opencodeoverlay deep-merged host-side with full override.docs/adr/README.md.It solves:
2. Intent
The intent of this PR is:
3. Scope
In Scope
Out of Scope
review.jsoncrefactor + the host-side full-override merge + the floor-diff warning) — a follow-up, sequenced after the in-flight cleanup/SAST tasks that touch the same rendering code.4. Verification
I verified this change by:
Commands run:
Results:
5. Screenshots / Evidence
integrations/opencode/config/opencode.jsonc(open mode).6. Risk Assessment
Risk level:
Potential risks:
Mitigation:
7. AI Usage Declaration
AI was used for:
Human verification:
8. Reviewer Focus
Please focus your review on:
🤖 Generated with Claude Code