diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 54c89cbf1..650e3ed85 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -354,7 +354,7 @@ All workflow toggles follow the **absent = enabled** pattern. If a key is missin | `workflow.nyquist_validation` | boolean | `true` | Test coverage mapping during plan-phase research | | `workflow.ui_phase` | boolean | `true` | Generate UI design contracts for frontend phases | | `workflow.ui_safety_gate` | boolean | `true` | Prompt to run /gsd-ui-phase for frontend phases during plan-phase | -| `workflow.assumption_delta` | boolean | `true` | Advisory architecture checkpoint during planning. When a phase makes something **plural, optional, or chosen** that used to be **singular, required, or derived** (e.g. a second auth method, a required field becoming optional, a constant becoming a parameter), the planner is prompted to re-ask whether the primary key / identity model still names the right thing (promote the new general representation vs. add it alongside). Non-blocking; fires only on a detected signal. Bare "or" is intentionally excluded (prose false-positives). Inspect a phase with `gsd query assumption-delta scan `. Added in #1561 | +| `workflow.assumption_delta` | boolean | `true` | Advisory architecture checkpoint during planning. When a phase makes something **plural, optional, or chosen** that used to be **singular, required, or derived** (e.g. a second auth method, a required field becoming optional, a constant becoming a parameter), the planner is prompted to re-ask whether the primary key / identity model still names the right thing (promote the new general representation vs. add it alongside). Non-blocking; fires only on a detected signal. Bare "or" is intentionally excluded (prose false-positives). Inspect a phase with `gsd_run query assumption-delta scan `. Added in #1561 | | `workflow.ui_review` | boolean | `true` | Run visual quality audit (`/gsd-ui-review`) after phase execution in autonomous mode. When `false`, the UI audit step is skipped. | | `workflow.node_repair` | boolean | `true` | Autonomous task repair on verification failure | | `workflow.node_repair_budget` | number | `2` | Max repair attempts per failed task | diff --git a/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md b/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md index aa96b534b..3263e878f 100644 --- a/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md +++ b/docs/adr/443-opus48-unified-effort-and-fast-mode-routing.md @@ -1,6 +1,6 @@ # ADR 443: Unified cross-provider effort controls and fast-mode-aware routing -- **Status:** Proposed (2026-05-28) +- **Status:** Accepted (2026-08-19) - **Date:** 2026-05-28 - **Tracking issue:** [#443](https://github.com/open-gsd/get-shit-done-redux/issues/443) @@ -37,6 +37,28 @@ The audit confirmed the cross-provider resolver/renderer/CLI machinery genuinely **Boundary.** #2313 owns the static/install-time effort channel for Codex (`model_reasoning_effort` in generated `~/.codex/agents/.toml`, plus a sync path) and explicitly places orchestrator effort-override drift outside its scope. That is the static channel this ADR already ships; the work above is the invocation-time channel it does not. +## Amendment (2026-08-19): Decision item 1 scoped to the operator surface; this ADR is ratified (#2475) + +Recorded as a dated section rather than by editing the amendment above or Decision item 1 itself, since ADRs here are append-only. + +**The remaining maintainer call has been made: unblock path (b), applied to Decision item 1 *only*.** The 2026-07-21 amendment above left this ADR `Proposed` on a single condition — that item 1's *orchestrator invocation override* (precedence step 1) gain a caller in shipped orchestration. It does not gain one. The scope is corrected instead. + +**Why the original (b) wording does not fit, and what replaces it.** The unblock condition offered (b) as *"if the ADR's intended scope is in fact limited to static install-time propagation, amend Decision items 1 and 6 to say so explicitly."* That sentence is now **false on both counts** and must not be adopted verbatim: item 6 has a live orchestration caller (#2296, `gsd-core/references/execute-phase-quota-recovery.md`, `@`-included into `execute-phase.md`), and #2481 delivered a live *invocation-time* channel in which the resolved cascade reaches a spawned host as an argv argument, gated by [ADR-1239](1239-gsd-embeddable-orchestration-engine.md)'s `effortSurface`. This ADR's scope is emphatically **not** limited to install-time propagation. What is narrowed is one precedence step, for a reason (b) did not anticipate. + +**What Decision item 1's invocation override is.** An **operator-facing surface**, not an orchestration channel. `gsd_run query resolve-execution --effort ` exists so a human — or an agent diagnosing routing — can ask *"what would this agent run at if effort were X?"* without mutating project or home configuration. That is its whole job, and it does it today. Shipped orchestration deliberately does **not** pass `--effort`: workflows resolve effort through precedence steps 2–5 (`effort.agent_overrides` → `effort.routing_tier_defaults` → `effort.default` → built-in), which is the configured, reviewable, per-project path. An override baked into a workflow would be an unconfigurable constant overriding the user's own configuration at the top of the cascade — the inverse of what step 1 is for. + +**Why no consumer was invented to satisfy the gate.** The unblock condition is a proxy for design maturity; wiring a caller purely to flip a status is the textbook case of a measure becoming a target, producing a number that looks better while the design gets worse. No user has asked for a per-invocation effort override, and #2475's actual complaint — reviewer CLIs silently inheriting a global effort default — is closed by the cascade→argv path, not by step 1. Adding an unrequested knob to ratify an ADR would also invert the very property [ADR-1239](1239-gsd-embeddable-orchestration-engine.md)'s `effortSurface` amendment claims for itself (*"a wired axis, not a declared-but-unconsumed one"*): a consumed-but-unrequested one is no better. + +**`--effort` is supported and is NOT deprecated.** Scoping it out of *orchestration* says nothing about its standing as a CLI flag. It ships, it is documented, it is covered by tests, and callers may rely on it. This paragraph exists so that a future reader — or a dead-code sweep — does not read "no orchestration caller" as "unused, remove it". + +**What is deliberately not built.** There is no per-run effort override for review lanes ("make *this* review cheap"). A user wanting that edits `effort.*`. Should a real need appear, it is a new enhancement to be judged on its merits — not a deferred obligation of this ADR, and not a request that was refused. + +**Status: `Proposed` → `Accepted`.** The corpus rule for ratifying a stale `Proposed` requires the decided mechanism to demonstrably exist in the tree. With item 1's decided scope narrowed to the operator surface, every Decision item now clears that bar: items 1–3 and 5 in the resolver, renderer, and `resolve-execution` output; item 4 in the static install-time propagation the 2026-07-17 audit confirmed end-to-end; item 6 in #2296's escalation caller; and the cascade's delivery to a spawned host in #2481's review-lane wiring. + +**The invariant this ratification rests on, and its guard.** Ratification is conditional on item 1 *staying* operator-only. `tests/effort-surface-axis.test.cjs` walks `gsd-core/workflows`, `gsd-core/references`, `agents`, and `commands` and fails if any of them invokes `resolve-execution … --effort` in either accepted shape (`--effort ` or `--effort=`). That guard was widened in this change: it previously matched only the space-separated form, so `--effort=low` would have passed it silently. **Wiring an orchestration caller therefore requires amending this ADR first** — the guard is the enforcement of this decision, not a snapshot of a gap awaiting closure. + +**Boundary — unchanged.** [ADR-2313](2313-codex-passive-model-posture.md) still owns the static/install-time channel; [ADR-1239](1239-gsd-embeddable-orchestration-engine.md)'s `effortSurface` amendment still owns the invocation-time argv channel. This amendment changes neither, and changes no runtime behavior at all. + ## Context ### Effort control and fast mode in Claude Opus 4.8 diff --git a/docs/adr/README.md b/docs/adr/README.md index 46ca2dcc2..604ce4c03 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -208,6 +208,7 @@ These govern the system as it stands. Cite these. | [ADR-218](218-release-version-validation.md) | Harden release-workflow version validation — reject leading zeros and pre-check npm | Accepted | — | | [ADR-227](227-input-validation-shape-not-just-type.md) | Input validation must check semantic shape, not just type | Accepted | — | | [ADR-415](415-prevent-stale-base-token-reintroduction.md) | Prevent stale-base reintroduction of retired runtime tokens | Accepted | — | +| [ADR-443](443-opus48-unified-effort-and-fast-mode-routing.md) | Unified cross-provider effort controls and fast-mode-aware routing | Accepted | — | | [ADR-452](452-eslint-lint-harness.md) | Adopt standard ESLint flat-config lint harness | Accepted | — | | [ADR-456](456-test-rigor-architecture.md) | Test-rigor architecture — deterministic scheduling, antagonistic tier, typed-surface mandate, and delete-bad-tests policy | Accepted | — | | [ADR-457](457-generated-cjs-single-source.md) | Generation model for `bin/lib/*.cjs` type safety | Accepted | — | @@ -265,7 +266,6 @@ Decided in principle, not yet ratified. Do not cite as settled architecture. | ADR | Title | Status | Read first | |-----|-------|--------|------------| | [ADR-230](230-introduce-next-integration-branch.md) | Introduce `next` as a long-lived integration branch | Proposed | — | -| [ADR-443](443-opus48-unified-effort-and-fast-mode-routing.md) | Unified cross-provider effort controls and fast-mode-aware routing | Proposed | — | | [ADR-612](612-bracket-phase-id-convention.md) | Bracket Phase-ID Convention | Proposed | — | | [ADR-660](660-release-from-next-head.md) | Release from the head of `next`; immutable release tags; `@next` dist-tag as the RC surface | Proposed | — | | [ADR-1143](1143-claude-orchestration-capability.md) | Claude orchestration capability — Workflow tool (ultracode) as a runtime-gated loop execution backend | Proposed | — | diff --git a/tests/effort-surface-axis.test.cjs b/tests/effort-surface-axis.test.cjs index 7402ef4bc..bfa51b087 100644 --- a/tests/effort-surface-axis.test.cjs +++ b/tests/effort-surface-axis.test.cjs @@ -496,10 +496,61 @@ describe('#2481 — ADR-443 mechanism callers, as they actually exist', () => { ); }); - test('Decision item 1 (invocation override) still has NO live caller', () => { - // Guards the corrected ADR-443 claim. If someone later wires --effort into a - // workflow, this fails and the ADR status text must be revisited — that is - // the point: the ADR must not silently drift back to being wrong. + /** + * Does this text invoke `resolve-execution` with an invocation-time effort override (#2475)? + * + * BOTH argument shapes, because the CLI accepts both: `--effort ` and `--effort=` + * (`gsd-core/bin/gsd-tools.cjs` — `a.slice('--effort='.length)`). The original matcher required + * `--effort\s`, so `--effort=low` — the terser form a workflow author is at least as likely to + * write — evaded it entirely, along with `--effort` at end-of-input. That hole mattered little + * while this guard merely SNAPSHOT a temporary gap; it matters a lot now that path (b) makes the + * guard the enforcement of a decision (ADR-443 amendment 2026-08-19). + * + * `[^\r\n]*` keeps the call and the flag on ONE line, so a `resolve-execution` on one line and an + * unrelated `--effort` on the next is not a false hit. The trailing `(?:[\s=]|$)` is what stops + * `--effortless` from matching: the character after `--effort` must be a delimiter or nothing. + * + * The unbounded quantifier is deliberate and safe here: the corpus scanned is maintainer-authored + * workflow, reference, and agent markdown — bounded prose, not adversarial input. + * + * DIVERGENCE RISK. This predicate independently models `gsd-tools.cjs`'s own argument parser; the + * two are not derived from one shared constant. If that parser ever accepts a THIRD spelling of + * `--effort`, this regex is the surface that must follow it — otherwise ADR-443's ratifying + * invariant silently stops holding while the guard still reports green. + */ + const EFFORT_CALLER_RE = /resolve-execution[^\r\n]*--effort(?:[\s=]|$)/; + const hasEffortCaller = (text) => EFFORT_CALLER_RE.test(String(text ?? '')); + + test('the item-1 matcher recognises every shape the CLI accepts, and nothing else', () => { + // Behavioral: the predicate is called with inputs and its verdict asserted. The equals form + // fails against the pre-#2475 matcher — it is the regression this sub-change closes. + for (const [label, text] of [ + ['space form', 'gsd_run query resolve-execution gsd-executor --effort low\n'], + ['equals form', 'gsd_run query resolve-execution gsd-executor --effort=low\n'], + ['bare trailing --effort', 'gsd_run query resolve-execution gsd-executor --effort\n'], + ['end of input, no newline', 'gsd_run query resolve-execution gsd-executor --effort'], + ['CRLF equals form', 'gsd_run query resolve-execution gsd-executor --effort=low\r\n'], + ]) { + assert.ok(hasEffortCaller(text), `must detect an item-1 caller written as: ${label}`); + } + + for (const [label, text] of [ + ['--effortless is a different word', 'resolve-execution gsd-executor --effortless\n'], + ['no effort argument at all', 'resolve-execution gsd-executor --host codex\n'], + ['--effort without resolve-execution', 'some-other-command --effort low\n'], + ["item 6's --attempt caller", 'resolve-execution gsd-executor --attempt 1\n'], + ['call and flag on different lines', 'resolve-execution\ngsd-executor --effort low\n'], + ['empty input', ''], + ]) { + assert.ok(!hasEffortCaller(text), `must NOT fire on: ${label}`); + } + }); + + test('Decision item 1 (invocation override) has no orchestration caller — by decision', () => { + // ADR-443's 2026-08-19 amendment settles this as path (b) FOR ITEM 1: the invocation-override + // step is an operator-facing CLI surface, deliberately not driven by shipped orchestration. + // So this is no longer a snapshot of a gap awaiting wiring — it is the invariant that keeps the + // ratified ADR true. A hit here is not "the ADR is stale", it is "the ADR must be amended first". const dirs = ['gsd-core/workflows', 'gsd-core/references', 'agents', 'commands']; const hits = []; const walk = (d) => { @@ -508,8 +559,7 @@ describe('#2481 — ADR-443 mechanism callers, as they actually exist', () => { for (const e of fs.readdirSync(abs, { withFileTypes: true })) { const full = path.join(abs, e.name); if (e.isDirectory()) walk(path.relative(REPO_ROOT, full)); - // eslint-disable-next-line local/no-unbounded-quantifier -- parses maintainer-authored workflow/reference/agent markdown, bounded prose, not adversarial input - else if (e.name.endsWith('.md') && /resolve-execution[^\r\n]*--effort\s/.test(fs.readFileSync(full, 'utf8'))) { + else if (e.name.endsWith('.md') && hasEffortCaller(fs.readFileSync(full, 'utf8'))) { hits.push(path.relative(REPO_ROOT, full)); } } @@ -517,8 +567,8 @@ describe('#2481 — ADR-443 mechanism callers, as they actually exist', () => { dirs.forEach(walk); assert.deepEqual( hits, [], - `ADR-443 records Decision item 1 as having no live caller; found: ${JSON.stringify(hits)}. ` + - 'Update the ADR-443 amendment before adding one.', + `ADR-443 records Decision item 1 as deliberately having no orchestration caller; found: ${JSON.stringify(hits)}. ` + + 'Amend ADR-443 before wiring one — the ADR is Accepted on the strength of this invariant.', ); }); });