feat(dynamic-workflow): tamper-evident integrity ledger for repair_cache and events

[modacker]

What changes

Dependency note: the pre-existing wait-polling race in checks/workspace-router.check.mjs (fixed by #44, which should merge first) will occasionally flake this PR's test run on fast machines — every failure we observed locally was that known race, never an integrity test.

Implements the integrity ledger proposed in #46: tamper-evident hash chains over the append-only surfaces (repair_cache inserts and events), so "the results reuse is about to trust are the ones earlier runs actually produced" becomes mechanically checkable. Closes #46.

Design constraints honored from the issue discussion: threat model is accidental mutation (agent mistakes), not adversaries; only append-only surfaces are chained; zero schema migration for existing databases; no new MCP tool; no change to any existing behavior or response shape except additive fields.

Design

• Two per-database chains: events (seq order) and repair_cache (rowid order). Row digest: sha256(prev:kind:key:body) over the exact stored JSON bytes; genesis prev is 64 zeros. Chain heads live in the existing settings table (integrity_events / integrity_repair as {head, upto}).
• Per-row ledger: a new integrity_rows(surface,pos,key,hash) table records each link, appended in the same transaction as the insert — this is what makes the first divergent row precisely locable (a single end-anchor alone cannot localize mid-chain tampering; we tried that design first and the test suite rejected it).
• Crash-safe writes: event() / saveRepairCandidate() wrap INSERT + chain advance in a transaction (the store transaction is now reentrant — engine.start() already wraps both calls in one). The first anchoring write implicitly commits pre-existing rows; later tampering with them is detected.
• Verify on demand: workflow_status (list form) always carries a light integrityHeads field; passing verifyIntegrity: true triggers a full recomputation and returns an integrity report per surface: {head,upto,verified,checked,unchained,firstDivergence}. verified is three-valued: true / false / null (nothing anchored yet). Rows beyond upto are reported as unchained — an honest window, not a false tamper verdict.
• Zero migration: the ledger table is CREATE TABLE IF NOT EXISTS; existing databases upgrade lazily on first write, old rows are implicitly committed by the first anchoring write.

Note: the list form of workflow_status changes from a bare array to {runs, integrityHeads, ...} — a bare JSON array cannot carry the added fields. The tool description documents the shape.

Test evidence

• New checks/integrity.check.mjs: 8 tests covering chain anchoring, byte-level tamper detection + healing, deleted-row detection, forged-head detection, honest unchained window, implicit commit of pre-existing rows, tool-layer schema/fields, and end-to-end repair flow.
• TDD order: tests landed first and verified red (8/8 fail on the missing API) before implementation; 6/8 green on first implementation; the 2 failures correctly rejected the weaker divergence-localization design, which was upgraded to the per-row ledger (documented above).
• Full plugin suite: 80/80 on green runs (72 + 8 new; the one intermittent failure is the pre-existing race from fix(dynamic-workflow): deterministic wait polling and checkpoint lineage in repair reuse #44's description, never an integrity test), packaged MCP smoke passes, npm run build byte-reproducible, repository validator green. One existing assertion migrated for the new list-form shape (workflow_status consumer in checks/workspace-router.check.mjs).
• Developed under an SDD + TDD discipline with an independent test cluster and implementation cluster converged against one contract (design record: sgmov/sih-engine sih/state/plan/integrity-ledger-parallel.md).

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[modacker]

Heads-up on the failing validate (ubuntu-latest): the single failure (not ok 182 — a fetch in another repository does not disable target attribution, cli-agent-bridge server.test.mjs:4404) is the known process-tree-termination flake in cli-agent-bridge — the same intermittent this PR's sibling run documented on #42, and what #43 fixes. Run 35297442863: exactly 1 of ~490 tests failed, in a different plugin than the one this PR touches. All dynamic-workflow checks (source-and-package, windows-process-lifecycle) and the integrity suite pass on Ubuntu.

[hetaoBackend]

Request changes for exact current head edc36cf.

Blocking CI regression:

• The required validate (ubuntu-latest) check is failing on the current head. The single failing test is plugins/Hylouis233/cli-agent-bridge/tests/server.test.mjs:4404: “a fetch in another repository does not disable target attribution”. It returns ok:false with orphanedProcesses:true, treeTerminated:false, terminationError:"process tree still appears alive after forceful termination", and quarantinePath=...quarantine; the assertion expected success. This is an actual repository test failure, not the skipped [code]smith check.

Do not merge while validate is red. Reproduce and fix the interaction between the new integrity/store changes and the cli-agent-bridge test/runtime, or demonstrate a correctly isolated flaky-test fix without weakening the fail-closed process-tree cleanup contract. Then rerun the exact-head Ubuntu validation and retain fresh green evidence.

Additional integrity semantics to clarify before approval:

• Store.verifyIntegrity() returns verified:true while reporting unchained > 0 for rows inserted after the ledger head (store.mjs:61-71, covered by checks/integrity.check.mjs:80-88). For a tamper-evident audit API, either make any unchained data fail closed or expose an explicit partial/unverified status that callers cannot mistake for a complete verification. Add tests for append-after-head, forged settings, and tail/middle deletion with the intended security semantics.
• The hash-chain record binds the row body and a derived key, but the public API must document that this is an in-database tamper-evidence signal, not an independent trust anchor. Do not present verified:true as proof against an attacker who can modify both SQLite data and integrity settings.

[code]smith is SKIPPED and is not evidence of correctness.

[hetaoBackend]

Follow-up review of current head 6df2db28640e901cd93121294ab0bdf342c4f7aa: changes are still needed before merge, despite the current green checks.

1. [P1] Record ownership/key mutations pass integrity verification. In src/store.mjs:65-68, verification fetches only the current row's body and hashes it with the ledger's old r.key. It never compares the current repair_cache.runId/id to that recorded identity. Event hashes also omit events.runId (:54).

   Reproduced in a disposable Store: create an event and repair candidate under run-original, then change only events.runId and repair_cache.runId/id to run-other/b. Leave the bodies, ledger and heads unchanged. Both surfaces still report verified: true, checked: 1, unchained: 0, although the original run has lost its records and the other run now returns them. This is an accidental data-mutation case within issue Design discussion: tamper-evident reuse results (hash-chained repair_cache/events) #46's stated threat model; it requires no modification of the integrity ledger.

   Bind actual record identity/ownership into each digest and derive/compare that identity from the current row during verification. Add regressions for changing each ownership/key field, alongside the existing body mutation/deletion tests.

2. [P2] Default workflow_status changes the existing response contract. src/tools.mjs:32 replaces the default list array with {runs, integrityHeads}. Existing callers using the list directly (for example .map) break; this PR itself updates one such consumer test. This is more than additive fields under the stated compatibility constraint. Preserve the existing default shape and expose the extended result through an explicit opt-in/new surface, or agree and document a versioned migration contract with consumer coverage.

Validation: the new integrity suite passes 8/8 locally, but does not cover the ownership/key mutation above. Current GitHub checks are green, so the older review's red-CI observation is no longer the current state.

I am not treating unchained > 0 together with verified: true as a separate implementation/spec violation: this PR explicitly defines verification of the anchored prefix. The public documentation should make that prefix-only meaning clear. The blocking integrity finding above has zero unchained rows and still returns a false assurance about record identity.

[modacker]

Thank you — both blockers are addressed at exact head 8aca0dd:

1. Record ownership/key mutations (your reproduced case): digests now bind record identity. Event digests cover events.runId (sha256(prev:event:<runId>:<seq>:<body>), ledger key <runId>:<seq>), and verifyIntegrity() re-derives the key from the current row's identity columns for every ledger record instead of trusting the stored key — your exact scenario (only events.runId / repair_cache.runId+id changed, bodies/ledger/heads intact) now returns verified:false on both surfaces with firstDivergence pointing at the original identity. Regression covers both surfaces plus restore-to-valid. The chain format is new (unmerged), so no migration is involved.
2. Default list shape: restored — workflow_status without runId returns the original bare array; the extended {runs, integrityHeads, integrity} object is opt-in via verifyIntegrity:true. The workspace-router consumer assertion is back to the upstream default shape.

On the prefix semantics: we kept the stricter fail-closed behavior (unchained > 0 → verified:false) rather than prefix-verified-with-disclosure — README and the tool description now state exactly that ("verification covers the anchored prefix; any unanchored row fails closed"), alongside the in-database-signal / not-a-trust-anchor boundary.

Validation at this head: full plugin suite 82/82, packaged MCP smoke 1/1, byte-reproducible rebuild. The earlier red validate run predates #43's merge; current head is green, and we additionally ran the complete 493-test repository gate on a local Ubuntu 24.04 box (Node 24) — 482 pass / 0 fail / 11 platform skips, including the cli-agent-bridge Linux paths. Re-requesting review.

[hetaoBackend]

Re-reviewed at 8aca0ddccd3ce34985f2af03bcdfd3d742ed0d37. The previous row-identity and default workflow_status compatibility findings are fixed, and the focused integrity suite passes (9/9).

One remaining issue before merging: unanchored records inserted into an older sequence gap are silently excluded from verification (src/store.mjs:68). unchained only counts positions greater than rec.upto, while verification only walks the existing ledger links.

I reproduced this independently for both events and repair_cache using a temporary SQLite database:

1. Seed legacy rows at positions 1 and 3.
2. Write position 4 through Store, anchoring positions 1, 3, and 4.
3. Simulate a restore/import that inserts a row at position 2.
4. Verify integrity: each table contains 4 rows, but both verdicts return verified: true, checked: 3, unchained: 0.

This contradicts the documented guarantee that any unanchored row fails closed. Please check coverage across the entire source table against ledger entries for the corresponding surface, rather than only counting the tail, and add regression cases for gap insertion on both surfaces.

Latest-head CI is green; this finding comes from the additional reproduction, not a failing existing test. No real model calls were involved.

[modacker]

Fixed at exact head 0572ac5: verifyIntegrity now checks coverage of the entire source table against ledger entries per surface — every row with position ≤ upto must carry a link. Your restore-into-gap repro (seeds at 1 and 3, anchor 1/3/4, import at 2) now returns verified:false with firstDivergence carrying the gap row's derived identity (run-b:2 / run-b/b); removing the gap row heals back to verified:true. Regression covers both surfaces. Full suite 83/83, packaged smoke 1/1, rebuild byte-reproducible.

Process note: head 174b344 was briefly pushed with this regression still red (a command chain committed without gating on the test result); 0572ac5 supersedes it green. Flagging it rather than hoping it goes unnoticed.

[hetaoBackend]

Re-reviewed exact head 0572ac502f534cad1ab9b3b92d8fc98965d2d821. The previous identity, default workflow_status compatibility, and older-sequence-gap findings are fixed. One integrity issue still needs addressing before merge.

Ordinary writes silently anchor externally inserted tail records

In plugins/hetaoBackend/mcode-dynamic-workflows/src/store.mjs:57, chainAdvance() scans every source row between the existing head's upto and the new insertion, even when a chain already exists. This makes the next ordinary Store write incorporate previously unanchored records into the ledger.

Independently reproduced on both events and repair_cache, using a temporary SQLite database:

1. Write one legitimate record through event() / saveRepairCandidate(), establishing the chain at position 1.
2. Insert another source-table record directly through SQL at position 2, without modifying the ledger or settings.
3. verifyIntegrity() correctly returns verified:false, checked:1, unchained:1 on both surfaces.
4. Write another legitimate record through the normal Store API at position 3.
5. Both surfaces now return verified:true, checked:3, unchained:0, firstDivergence:null. The externally inserted record is still present.

Thus a record that was correctly rejected by verification becomes accepted solely because unrelated normal activity appends another record. The documented legacy behavior permits implicit adoption on the first anchoring write; it should not silently repeat after an anchor already exists.

Please restrict legacy bulk anchoring to initial chain creation. For subsequent writes, detect unexpected unanchored records and preserve the failure (or require an explicit recovery/re-anchoring operation), rather than automatically incorporating them. Add regressions for both surfaces covering raw tail insertion followed by a normal API write, alongside the existing immediate-verification test.

Validation at this exact head: plugin checks 83/83, packaged MCP smoke 1/1, byte-identical rebuild, and clean-tree repository validator pass. Current GitHub checks are 5 passed / 0 failed / 1 skipped. The finding comes from an additional independent Store/SQLite reproduction, not a failing existing test; no real model calls were involved.

[modacker]

Rebased onto main after #50 (and #45's merge); exact head is now adc0f19. Validate now runs the stabilized suite — no cli-agent-bridge integration flakes in the default discovery set. Full plugin suite green at this head (92/92, incl. #45's claims/examples checks now in the baseline), packaged smoke 1/1, rebuild byte-reproducible.

[modacker]

Fixed at exact head 51c7150 (rebased after #50): bulk adoption of pre-existing rows now happens only at initial chain creation — chainAdvance anchors exactly the write's own position once a head exists. Your five-step repro now behaves as: raw insert → verified:false (unchained); normal write → still verified:false, with the injected row reported as an in-range gap carrying its derived identity (run-raw:2 / run-raw/raw, checked=2, unchained=0) — never silently accepted; removing the injected row heals. Regression covers both surfaces end-to-end. Suite 93/93, packaged smoke 1/1, rebuild byte-reproducible.

[hetaoBackend]

Request changes for exact current head 51c71506a71bf2bdd4d597f4b80401176b2eeba1.

The core integrity logic and the previous Ubuntu CI regression are fixed, but two public-contract gaps remain:

1. Document the trust boundary. src/store.mjs:22-30 stores the source data, integrity_rows, and the integrity head/settings in the same SQLite database. The README near README.md:57 and the public workflow_status description in src/tools.mjs:16 still present this as tamper evidence without stating that it is in-database tamper evidence, not an independent trust anchor. A party able to rewrite the whole SQLite database can also recompute the ledger/head; verified:true must not be documented or interpreted as proof against that attacker. Add this limitation to the public README/tool contract.

2. Complete deletion regressions on both surfaces. The implementation at src/store.mjs:77-89 appears to fail closed for missing covered rows, but checks/integrity.check.mjs:86-95 only deletes the first of two event rows. Add distinct tail-deletion and middle-deletion cases, and cover both events and repair_cache, so later refactors cannot silently weaken the intended semantics.

Exact-head validate (ubuntu-latest), source/package, Windows lifecycle, and CodeQL checks are green. [code]smith is skipped and was not used as evidence. Once the trust-model disclaimer and deletion regressions are present on a new head, this should be ready for another review.

[modacker]

Both findings addressed at exact head d248bfa (rebased after #49's merge):

1. Trust boundary documented in the public contract. The README's data section and the workflow_status description now state it explicitly: the ledger lives in the same SQLite database as the data it covers — in-database tamper evidence, not an independent trust anchor; a party able to rewrite the whole database can also recompute the ledger and heads, and verified:true must not be read as proof against that attacker. (What it does catch: accidental edits, partial rewrites, tooling mistakes.)
2. Deletion regressions completed: four distinct cases — tail and middle deletion, on both events and repair_cache — with row-identity keys, actualHead:null, expected-head placement (head vs intermediate hash), and heal-on-restore assertions.

Validation at this head: plugin suite 117/117, packaged MCP smoke 1/1, byte-reproducible rebuild, and the full 345-test repository gate on a local Ubuntu box (345/345, 0 failures). Ready for another review.