Files
msd-core/docs/adr/1411-resolution-provenance.md
Tom Boucher b0c774c2e3 feat(#1416): formalize Resolution convention + agent-skills value envelope (Resolution Provenance P3) (#1425)
Narrows P3 of ADR-1411 (Resolution Provenance, epic #1411) based on an
adversarial fit-analysis that showed a single Resolution<T> envelope adopted
by agent-skills, capability-state, and capability-writer fails the deletion
test: configured/reason are meaningless for capability verbs, and
capability-writer's errors[] (operation-not-applied) cannot fold into
warnings[]. The only genuinely shared seam is warnings: string[].

Changes:

- src/resolution.cts: new pure types+builder leaf — exports Resolution<T>
  {value, configured, reason, warnings}, makeResolution<T>() builder, and
  AgentSkillsValue {block, skills_count}. No other src/ imports.

- src/init.cts: cmdAgentSkills --json IR gains additive value:{block,
  skills_count} field (built via makeResolution). All existing flat fields
  (agent_type, block, skills_count, warnings, configured, reason, source,
  degraded) are retained unchanged for back-compat.

- src/capability-state.cts: doc comment on ResolveCapabilityRuntimeStateResult
  naming it the canonical read-verb envelope. No JSON change.

- src/capability-writer.cts: doc comment on SetCapabilityStateResult naming it
  the canonical mutation-verb result (warnings=advisory, errors=operation-
  not-applied). No JSON change.

- CONTEXT.md: new ### Resolution Convention glossary entry after
  ### Resolution Provenance.

- docs/adr/1411-resolution-provenance.md: P3 narrowing amendment appended.

- tests/resolution.test.cjs: 9 unit tests for makeResolution (new).
- tests/agent-skills.test.cjs: 2 P3 tests for value.block/value.skills_count
  and back-compat of all flat fields.

All 277 tests pass (5 suites). npm run lint clean. All lint checks pass.

Part of #1411

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-18 07:44:52 -04:00

8.5 KiB
Raw Blame History

Resolution must report provenance, not fall open silently

  • Status: Accepted
  • Date: 2026-06-17

Context

A verb resolves config (or a skill set, or a planning path) from the invoking cwd / GSD_WORKSTREAM / stored workstream pointer. When that ambient context is "off" — a descendant subdirectory with no .planning/, or a workstream with no scoped config — resolution silently falls open to bare defaults, the verb succeeds with empty output and no signal, and a downstream subagent plans or verifies without its configured context. The gap is invisible: output is still produced.

We have shipped the same fix-shape ≈11 times between April and June 2026 — anchor to the project root / fall back to the root config instead of defaults / bolt a diagnostic onto one verb. The recurrence is concentrated, not scattered:

  • loadConfig is a 9-patch try/catch ladder (#315, #443, #910, #1683, #2517, #2714, #3023, #3024, #3523). Each "config fell to defaults" bug adds a branch, and the returned object is the same shape whether it found real config or bare defaults — so every caller that cares re-detects degradation by sniffing the contents.
  • The agent-skills verb received this exact fix twice in two weeks: #1374/#1376 (the warnings[] field) and then #1366 (PR #1408).
  • The diagnostic half is hand-rolled eight ways across seven files; the I/O Module's output() has no notion of a "degraded" result.
  • The walk-up to the project root exists three-to-four times; PR #1408 adds a weaker fourth (resolvePlanningCwd) because the canonical findProjectRoot (Project-Root Resolution Module) skips the plain single-repo-descendant case.

The #1366 trigger

gsd-tools query agent-skills <agent> resolved a configured agent's <agent_skills> block to empty with no diagnostic under two invocation-context drifts: (1) invoked from a descendant subdirectory with no .planning/, config fell through to bare defaults → agent_skills was {}; (2) GSD_WORKSTREAM pointed at a workstream with no scoped config → the same fall-through. In both cases the verb exited 0 and emitted an empty block, so a planner/checker subagent planned or verified without its configured skill/rule context, invisibly.

The generalizing precedent

ADR-227 established that input validation at a trust boundary must check semantic shape, not just type, and surface coercion rather than propagate a contractually-invalid value. This ADR is the analog for the resolution side of the same trust boundary: looking a value up from ambient context (cwd, env, a stored pointer) is itself a trust boundary, and silently substituting defaults when the lookup misses is the resolution-side equivalent of propagating a garbage value — the caller cannot tell a real answer from a degraded one. CONTEXT.md's Planning Path Projection Module already states the rule for the SDK path-projection seam — "invalid workspace context is a validation error at this seam rather than a silent fallback" — but the CJS loadConfig never adopted it.

Decision

Context resolution at a trust boundary — reading config, anchoring to a project root, resolving a workstream — MUST report its provenance. A resolver may fall back, but the fallback must be a visible value, not a silent substitution. Three sub-rules:

  1. Deterministic anchoring. Resolve the project root through one walk-up module. Resolution MUST NOT depend on an arbitrary descendant cwd. The single owner is the Project-Root Resolution Module; ad-hoc walk-ups (e.g. resolvePlanningCwd) are retired into it.
  2. Provenance, not a bare value. A resolver returns what it resolved and where it came from. Callers branch on the provenance field, never on the resolved contents, to detect degradation.
  3. Visible degradation. A configured input that resolves empty MUST emit a diagnostic. "Not configured" and "configured-but-resolved-empty" MUST be distinguishable in the output contract.

Concretely, the principle binds three seams:

  • Config Loader Module — loadConfig exposes a ConfigResolution { config, source: 'workstream' | 'root' | 'global-defaults' | 'builtin-defaults', degraded: boolean }. Introduced additively (loadConfigResolved) so the ~16 existing loadConfig call sites, SDK parity, and the generated .cjs are unaffected until they opt in.
  • Project-Root Resolution Module — absorbs the nearest-.planning/ ancestor as a first-class heuristic; resolvePlanningCwd and any sibling walk-up are deleted.
  • I/O Module — a shared Resolution<T> { value, configured, reason, warnings } envelope; output() carries degradation so the eight hand-rolled warnings[] shapes converge on one.

A configured input that resolves empty without a reason is a CI-guarded regression (grandfather burn-down, mirroring the no-adhoc-markdown-parsing rule).

Consequences

Bug classes avoided

  • Silent context drop — a planner/checker subagent planning or verifying without its configured skills (the #1366 / #1374 class).
  • N callers re-sniffing — every consumer re-deriving "did this fall open?" from config contents instead of reading one field.
  • Walk-up drift — a fourth or fifth project-root resolver diverging from the canonical one.

Cost

  • loadConfig's result type grows — mitigated by the additive loadConfigResolved; callers migrate incrementally.
  • One envelope to learn; ~18 verbs migrate onto it across phases P3–P4.

Tradeoff

As in ADR-227, resolution may still fall back to preserve continuity — a missing workstream config should not abort the verb. The difference is that the fallback is now a visible value plus an opt-in warning, never a silent success. Fields where a miss is genuinely fatal may throw; that is a per-call decision, not the general rule.

Alternatives considered

Per-verb patching (status quo)

Rejected. The same fix-shape regenerated ≈11 times because each patch fixed one call site without changing the policy that the resolver fails open and hides which branch fired.

Throw on a resolution miss

Rejected, for ADR-227's reason: throwing breaks pipeline continuity. A missing workstream config must not abort query agent-skills. Visible provenance preserves continuity and visibility.

Deterministic anchoring only (no provenance)

Rejected. Fixing cwd/workstream drift removes the most common trigger but leaves callers re-sniffing contents and the diagnostic hand-rolled per verb — the bug class would keep regenerating at the next new consumer.

  • Epic: #1411 (Resolution Provenance) · This ADR (P0): #1412
  • Supersedes the tactical fix in PR #1408 (closed) — its resolvePlanningCwd and local AgentSkillsReason/AgentSkillsDiagnostics are redelivered through the seams above in P1–P3.
  • Builds on: ADR-227 (input validation shape), ADR-0004 (Planning Workspace Module), ADR-0006 (Planning Path Projection Module).
  • Prior recurrences of this class: #1374/#1376, #1683, #991, #2714, #2638, #3523, #2652, #2791, #2555, #2623, #3196.

Amendment — 2026-06-18: P3 narrowed (the shared envelope is not a real seam)

The original P3 plan was a single Resolution<T> { value, configured, reason, warnings } envelope adopted by agent-skills, capability-state, and capability-writer. An adversarial fit-analysis showed this fails the deletion test: configured/reason are meaningless for the capability read/mutation verbs, and capability-writer's errors[] (operation-not-applied) is load-bearing and cannot fold into warnings[] (advisory). The only genuinely shared seam across the three is warnings: string[].

P3 is therefore narrowed to an honest convention rather than a forced generic:

  • Resolution<T> { value, configured, reason, warnings } (src/resolution.cts) is the canonical shape for config-interpreting read verbs. agent-skills is the first adopter — the value field is added additively to its --json IR with the flat fields retained for back-compat; source/degraded remain config-provenance extras.
  • Capability verbs keep their existing shapes, named explicitly: read = { runtimeConfigDir, capabilities, warnings? }; mutation = { capabilities, warnings, errors }.
  • The shared contract is documented, not forced: read verbs expose warnings[]; mutation verbs expose warnings[] + errors[]; configured/reason appear only on config-interpreting read verbs.

Recurrence prevention does not depend on a shared envelope — it is delivered by P4's CI guard (a configured input resolving empty must carry a reason). (#1416)