feat(impact-certificate): certify a change's cross-boundary reach before it lands (spec-store arc, change 3) - #181
Merged
Conversation
…ore it lands (spec-store arc, change 3) Adds change_impact_certificate (MCP tool + `openlore impact-certificate` CLI): for the current diff, emit ONE deterministic, conclusion-shaped certificate — blast radius (reused from blast_radius), the paths the change NEWLY OPENS into each declared covering surface, drifted specs, and tests to run. Newly-opened-path detection is differential and needs no full rebuild and no (unbuilt) incremental graph: a new call edge can only originate from a changed file, so re-parse only the changed files at base vs working tree, take each caller's added/removed callee names, resolve them to canonical ids by unique-name match, and adjust the canonical adjacency both ways (post = canonical + added - removed, pre = canonical - added + removed). A node that reaches a surface in post but not pre is newly able to — the path opened. The certificate decays via the existing code-anchored freshness lease: it is anchored to the touched symbols, persisted under .openlore/impact-certificates/, and the spec-store health check re-fires a stale one as a finding. Advisory by default; opt-in blocking only on a configured surface severity. No LLM. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ogfood report; clean path naming - Merge the change-impact-certificate ADDED requirements into the canonical mcp-handlers + cli specs (CoveringSurfaceDeclaration, NewlyOpenedPathDetection, ChangeImpactCertificate, ImpactCertificateDecaysWithLease, ImpactCertificateCommand), each annotated with the scoped differential-edge-delta deviation. - Flip proposal + tasks to IMPLEMENTED; mark the spec-store arc COMPLETE (all 3 shipped). - Add CLAUDE.md tool-table row + DOGFOOD-change-impact-certificate.md (real e2e on this repo). - Polish: a newly-added caller absent from the index now renders its bare symbol name in the opening path, not its path-based node id. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…iles, no-throw decay Two correctness bugs found by adversarial review + real-input e2e, both from computeEdgeDelta diverging from the sibling structural_diff that gets these right: - RENAME false positive (HIGH): the diff dropped getChangedFiles' oldPath, so a renamed file's old content was read by its NEW path (git show fails) → every pre-existing call looked "added" → a pure rename falsely reported a newly-opened path (and would have wrongly BLOCKED the commit under block:["critical"]). Fix: ChangedFileEntry carries oldPath/status; old content reads from oldPath ?? path. - UNTRACKED false negative (MEDIUM): git diff excludes untracked files, so a brand-new (un-git-added) file opening a path into a surface was silently missed — the certificate certified "no new reach" while a real critical opening existed. Fix: fold in `git ls-files --others --exclude-standard` (as structural_diff does). - NO-THROW gap (MEDIUM): recheckCertificate / recheckPersistedCertificates could throw (corrupt target anchor graph, wrong-typed lease) out of the contractually no-throw handleSpecStoreStatus. Fix: catch at the source + a try/catch boundary at the spec-store call site; an unverifiable certificate is conservatively stale. Regression-tested against a real temp git repo + the real CallGraphBuilder snapshot (17 → 21 tests: rename opens nothing, untracked detected, in-place edit detected, corrupt cert never throws). Full CI-equivalent suite: 4,380 passed / 2 skipped. Dogfood Round 2 + the diff-completeness guarantee recorded in the specs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rface miss; full docs + CLI tests Second adversarial round (two reviewers + real-input probes) on PR #181. Correctness: - HOMONYM PHANTOM OPENING (HIGH): resolving an added call's callee by NAME against the canonical graph mis-bound a LOCAL helper sharing a surface symbol's name to the canonical surface - a phantom newly-opened path that falsely tripped the critical block-gate (reproduced e2e). Fix: key changed-file calls by their snapshot-internal resolved id and only name-resolve callees external to the snapshot, honoring the snapshot's own local binding. (Decision 97c22605.) - SAME-DIFF SURFACE MEMBER MISSED (MEDIUM): a surface symbol added in the same diff was unresolvable (canonical-only), silently downgrading a critical opening to a warn. Fix: resolve surfaces over canonical + post-change snapshot nodes; compute the edge delta before surface resolution. - Three stray NUL bytes (accidental key separators) made the file read as binary and hid it from grep - replaced with the unicode escape (identical runtime, valid UTF-8). Docs (previously only in CLAUDE.md): change_impact_certificate / openlore impact-certificate now documented in docs/mcp-tools.md (table + prose + parameters + finding codes), docs/cli-reference.md (table + command section), README.md, and docs/federation.md. Tests: new src/cli/commands/impact-certificate.test.ts (hook install/uninstall + advisory/blocking exit codes), a dispatchTool MCP-path reachability test, and regression tests for the homonym + same-diff-surface bugs against a real temp git repo. Full CI-equivalent suite: 4,399 passed / 2 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ng (3rd adversarial pass) Third adversarial round on PR #181 found two HIGH correctness bugs + integrity gaps. - BASE-REF DIVERGENCE (HIGH): getChangedFiles diffs against the MERGE-BASE (three-dot base...HEAD) but the differential read old content from the base-ref TIP. When the base branch advanced, the certificate's two halves diffed against different commits -> phantom or missed openings. Fix: read old content from git merge-base(base, HEAD). Reproduced + pinned in a real temp git repo. - WRONG-TYPED SEVERITY (HIGH): an out-of-enum surface severity made highestSurfaceSeverity come out null (NaN SEVERITY_RANK index), breaking the block-signal contract. Fix: coerce to 'warn' in surfacesFromConfig (+ ?? 0 guard). Reproduced e2e (was null, now 'warn'). - Duplicate surface names (collided in the findings map, dropped a severity) and empty/whitespace names are now dropped; a member with both symbol+file resolves both; the newly-opened-paths sort is a total order (deterministic top-N); per-surface path truncation reports the TRUE count + a caveat (no silent truncation); a large-diff caveat is emitted; and a new/untracked file (no indexed symbol) gets a FILE-level lease anchor so the certificate actually decays for it. - Fixed two stray SOH bytes (raw \x01 sort separators) that re-flagged the file as binary; replaced with the escape. Docs: docs/configuration.md now documents the impactCertificate config (surfaces, members, severity, block) - the last doc surface that lacked it. Tests: merge-base baseline (real git repo), severity/duplicate/empty-name coercion, both-member resolution, file-level-anchor decay. Suite: 4,405 passed / 2 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-server integration gap (4th pass) Fourth adversarial pass (doc-accuracy + integration-coverage reviewer + a real large-diff perf probe). No new runtime bugs; the differential/decay/config/finding codes were all confirmed accurate. Fixed doc inaccuracies + one integration gap. - DOC ACCURACY: three rounds of fixes settled the implementation on a differential edge-delta over changed files, but five normative spec/proposal BODIES still asserted the un-shipped "incremental dependency graph" as the mechanism (contradicting the code and their own notes). Reworded all five to be mechanism-neutral / name the differential: canonical mcp-handlers requirement, the change-delta spec (added a note where none existed), proposal items 2 and "Application to OpenLore", and tasks.md item 2. Extended the lease note to record that new/untracked files get a file-level anchor. - INTEGRATION COVERAGE: the spec-12 conformance test ran the live MCP stdio server but its ListTools check was one-directional (advertised subset of known). Made it BIDIRECTIONAL (every defined tool is advertised) + a positive assertion that change_impact_certificate is advertised. Verified: 7/7 conformance tests pass, so the tool is now confirmed reachable end-to-end through the real MCP server, not just the unit-level dispatchTool path. The bidirectional check protects all tools. - Perf probe (real input): a 300-file diff completes in ~8.8s (300 git-show + 2 tree-sitter builds), 300/300 openings detected, no hang - validating the round-4 >200-file caveat (capping is correctly avoided; it would miss openings). - docs/cli-reference.md: added the --uninstall-hook example for symmetry. Suite: 4,405 passed / 2 skipped; conformance integration: 7 passed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rtificate # Conflicts: # README.md # docs/cli-reference.md # docs/governance-dogfooding.md # docs/mcp-tools.md # openspec/specs/cli/spec.md # src/cli/commands/mcp.ts
clay-good
changed the base branch from
feat/working-set-context-briefing
to
main
June 22, 2026 00:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Third and final change of the spec-store integration arc (
openspec/changes/SPEC-STORE-INTEGRATION.md), stacked on #180 (change 2). Adds the change-impact certificate: for the current diff, OpenLore emits ONE deterministic, conclusion-shaped certificate of what the change touches — before it lands.What it does
change_impact_certificate(MCP tool) +openlore impact-certificate(CLI) compose, for a proposed change:blast_radius..openlore/impact-certificates/, and the spec-store health check re-fires a stale one as acertificate-stalefinding. An expired certificate is never presented as silently still-true.Advisory by default (never blocks); opt-in blocking only on a configured surface severity (e.g.
impactCertificate.block: ["critical"]). No LLM — north starc6d1ad07holds.The differential, without the unbuilt incremental graph
The proposal specified deriving the post-change graph via
add-watch-incremental-dependency-graph, which is still a DRAFT/unbuilt. A new call edge can only originate from a changed file, so this re-parses only the changed files at base vs working tree (the same bounded primitivestructural_diffuses), resolves the added/removed callee names to canonical ids by unique-name match, and adjusts the canonical adjacency both ways:A node that reaches a surface in
postbut notpreis newly able to — and the opening is attributed to the exact added edge, with its shortest path named. No full rebuild, no dependency on the unbuilt graph. Ambiguous added callees are reported (unresolved-added-call), never guessed. Recorded as decision187224b0.Verification
impact-certificate.test.ts— 17 tests: surface resolution (file+symbol, unresolved→finding, ambiguous→no-guess), both differential scenarios (opens a 2-hop path / touches only existing callers → none) + direct + already-reachable + empty, the block gate, conclusion-shape + contract classification, and decay fresh→stale against a real on-disk edge store + the spec-store health-check re-fire.dogfoodNewlyOpensSurface → validateSpecStoreConfig, blocked the hook at exit 1 underblock: ["critical"]and stayed advisory (exit 0) without it, persisted 31 lease anchors, and turned a certificate stale on an anchored-symbol edit. Seeopenspec/changes/DOGFOOD-change-impact-certificate.md.vitest run src examples(integration excluded) — 4,376 passed / 2 skipped.Surface registration
New tool joins the opt-in
federationpreset only (out of minimal/navigation/memory). Tool count 62→63; tools/list budget, doc size/count figures, presets/contract/tool-driver guards all updated consciously.Adversarial hardening (round 2)
Two independent reviewers + real-input e2e probes found and fixed two correctness bugs in the changed-file plumbing (both from
computeEdgeDeltadiverging from the siblingstructural_diff) plus one no-throw gap:oldPath, so a renamed file's pre-existing calls all looked added — a puregit mvfalsely reported a newly-opened critical path and would have wrongly blocked the commit underblock: ["critical"]. Fixed: read old content fromoldPath ?? path.git diffexcludes untracked files, so a brand-new file opening a critical surface was certified as "no new reach." Fixed: fold ingit ls-files --others --exclude-standard(asstructural_diffdoes).handleSpecStoreStatus. Fixed: catch at the source + a boundary at the call site; an unverifiable certificate is conservativelystale.All three reproduced with real inputs, fixed, and pinned by 4 new regression tests against a real temp git repo + the real
CallGraphBuilder(17 → 21 tests). See the Round 2 section ofDOGFOOD-change-impact-certificate.md.Adversarial hardening (round 3 — second pass)
A second adversarial round (two fresh reviewers + new real-input probes) found two more correctness bugs, a file-hygiene defect, and doc/test gaps. All fixed and regression-tested.
grep; replaced with the equivalent escape.CLAUDE.md; now fully documented indocs/mcp-tools.md(table + prose + parameters + finding codes),docs/cli-reference.md,README.md, anddocs/federation.md.dispatchToolMCP-path reachability test, and homonym + same-diff regression tests against a real temp git repo. Full suite: 4,399 passed / 2 skipped.Adversarial hardening (round 4 - third pass)
A third adversarial pass (scale/determinism/correctness reviewer + new real-input probes) found two more HIGH correctness bugs and several integrity gaps. All fixed + regression-tested.
getChangedFilesdiffs against the merge-base (three-dot) but the differential read old content from the base-ref tip - so when the base branch advanced, the certificate's two halves diffed against different commits (phantom/missed openings). Fixed: read old content fromgit merge-base(base, HEAD); pinned in a real temp git repo.severitymadehighestSurfaceSeveritycome outnull(NaN rank index), breaking the block-signal contract. Fixed: coerce towarninsurfacesFromConfig(reproduced e2e: wasnull, nowwarn).symbol+fileresolves both; total-order path sort (deterministic top-N); per-surface truncation now reports the true count + a caveat (no silent truncation); large-diff caveat; and a new/untracked file now gets a file-level lease anchor so the certificate actually decays for it.docs/configuration.mdnow documents theimpactCertificateconfig (surfaces, members, severity, block) - the last doc surface that lacked it.Round 5 - doc accuracy + MCP-server integration (4th pass)
A fourth pass (spec/doc-accuracy + integration-coverage reviewer + a real large-diff perf probe) found no new runtime bugs - the differential, decay, config hardening, and finding codes were all confirmed accurate - and fixed documentation + integration-coverage gaps:
change_impact_certificateis advertised — 7/7 conformance tests pass, confirming the tool is reachable end-to-end through the real MCP server (not just unit dispatch). Protects all tools.--uninstall-hookdoc example.🤖 Generated with Claude Code