Skip to content

docs(audits): census of same-key/per-row-value beforeUpdate rewrites — zero in-repo, guard precision 0/5 - #15301

Merged
zhuangjianguo merged 7 commits into
mainfrom
claude/issue-14744-multi-update-per-row-value-census
Sep 4, 2026
Merged

docs(audits): census of same-key/per-row-value beforeUpdate rewrites — zero in-repo, guard precision 0/5#15301
zhuangjianguo merged 7 commits into
mainfrom
claude/issue-14744-multi-update-per-row-value-census

Conversation

@claude

@claude claude Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Part of #14744

Measurement only. ⛔ No guard is implemented, no write shape is changed, no ADR is opened or amended, and packages/objectql/src/engine.ts and packages/objectql/src/multi-update-hook-key-divergence.ts were read but not edited — per triage's terminal scoping on the card, which put the fix on the human floor because closing this residue reverses ADR-0058 Addendum II D3.

Answer

The in-repo population of same-key / per-row-value beforeUpdate rewrites is ZERO, across 23 production registration sites, derived two independent ways that agree on every subject.

The candidate provenance guard fires on 5 of the 23, and none of the 5 is an instance — precision 0/5 on the population it would ship to catch. The structural reason is the main finding: the guard's predicate conflates reading the pre-image to decide whether to write with reading it to compute what to write, and only the second is this residue. Every in-repo pre-image read is the first kind, and a whether-decision that differs between rows is a key-set divergence — already refused by #14099. So on today's tree the guard would refuse only batches #14099 already refuses, plus honest ones.

What is added

path what
docs/audits/2026-09-multi-update-per-row-value-census.md the census: predicate, both instruments, results, blind spots, and what the follow-on decision needs
scripts/audits/14744-before-update-per-row-value-census.mjs instrument A — static AST enumeration + taint classification (--self-test)
scripts/audits/14744-before-update-per-row-value-probe.mjs instrument B — runtime behavioural probe over the real engine (--out path.json)
scripts/pm/dispatch-gates.mjs +1 line — registers instrument A's runSelfTest in COMPOUND_ANCHOR_LEDGER (see Patch round)

Instrument A — static

An AST walk, not grep, and the difference is load-bearing: on this tree a single-line git grep "registerHook('beforeUpdate'" finds 14 sites while 46 further registerHook( sites are multi-line or take the event through a variable. A line-oriented census would have under-counted by construction.

Classification is taint over an alias lattice that propagates across callssys_stamp_audit_update writes the payload only through stampData(hookCtx.input.data, …), so the first revision, which followed only the context, scored the most important handler in the population as "writes nothing". Recorded in the doc because the mistake is available to any re-derivation.

--self-test is 10/10 in both directions, including the card's own pinned residue as a firing positive control and the guard's over-fire shape as a negative one.

Instrument B — runtime, on the real handlers

Boots the real ObjectQL against a stub driver and dispatches handlers per row of a genuine multi: true update, reading what actually reaches driver.updateMany. Four of eight subjects are the shipped handlers themselves, imported and dispatched unmodified (task_logic, and the plugin-email / plugin-sharing / plugin-webhooks provenance stamps); the audit stamp and pinyin projection are labelled replicas, following the pin suite's own convention.

Two scenarios per subject — rows that disagree on the pre-image field, and rows that agree — because one batch cannot separate the residue from the clock. The guard is evaluated as an observer, never as enforcement.

subject real? verdict guard fires
POSITIVE CONTROL — the card's pinned residue replica INSTANCE yes
sys_stamp_audit_update replica NONDETERMINISTIC no
examples/app-todo task_logic real CAUGHT_BY_14099 yes
plugin-email template provenance real CAUGHT_BY_14099 yes
plugin-sharing rule provenance real CAUGHT_BY_14099 yes
plugin-webhooks provenance real CAUGHT_BY_14099 yes
pinyin companion projection replica ROW_INVARIANT no
NEGATIVE CONTROL — reads, never writes replica NOT_A_PAYLOAD_WRITER no

The positive control also reproduces the defect end-to-end on current main: row a derived priority: 'high', row b derived 'low', the single SET clause carried 'low', and both rows stored 'low'. That confirms the card's correction to the #14099 ruling's prose — the surviving value is the last dispatch's, not the first.

Blind spots, stated rather than assumed away

Enumerated by the instrument itself, not asserted: 7 production registrations take a non-literal event argument (each read by hand — webhook-headers-gate.ts:313 is a real beforeUpdate the walk does not count, and its handler only throws); record-change-trigger.ts:299 binds beforeUpdate from user-authored flow metadata, the one open door, resolved only by source reading; no metadata-declared hooks exist in-tree; 0 TypeScript files mentioning the event outside the walked roots. ⛔ And the largest blind spot is not in the tree at all — a zero in-repo is not a zero in the field, and the duly_task corruption that motivated #14099 was measured downstream.

Patch round — the gate this diff's own KIND tripped

Lint & Repo Gates went red at fa01034f8 on this PR's own failure:

✗ every compound self-test NAME the anchor matches is classified in COMPOUND_ANCHOR_LEDGER
  — unlisted: scripts/audits/14744-before-update-per-row-value-census.mjs::runSelfTest
✗ dispatch-gates self-test: 1 of 1371 case(s) failed.

Reproduced verbatim before any fix, then fixed, then shown passing — the same line now reads , verdict ✓ dispatch-gates self-test: 1371 cases pass, exit 0, zero in the whole run. pnpm check:pm-dispatch-gates exit 0.

Fixed by registering the self-test (accidental: false), classified on the same evidence the ledger uses for the two existing runSelfTest rows it calls genuine: not exported, reached only from the file's --self-test guard, spawns no other script's self-test, and holds inline fixtures that should be masked. ⛔ The self-test was not renamed or removed to dodge the anchor — the ledger's own docblock refuses that explicitly, and doing so would have gutted this card's evidence, since that self-test is the 10/10 both-directions run carrying the firing positive control.

Why the original 25-family derivation could not have caught this. dispatch-gates.mjs prints this class under Convention-triggered gates, naming "adds or edits a GATE SCRIPT (a file some discovered check family runs)" — judged from the change's KIND, not from a path filter. The tool maps paths → gates, so a path derivation cannot reach it by construction. The 25-family list reported before the patch round was honestly derived and correct for what a path derivation can see.

Verification

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no path arguments) re-derived 28 runnable families for the now-4-path change set — up from 25, the gate-script edit adding check:declared-population-live, check:pm-dispatch-gates and check:ratchet-remedy-authority. All 28 exit 0 at af1826708, exit codes captured by redirecting to a file before any pipe.

⚠️ One family was NOT MEASURED and is reported as such rather than as a pass or a failure. After merging main, check:doc-formula-expressions exited 3 with PREREQUISITE NOT MET — @objectstack/formula is not built — which the gate itself labels "Nothing was measured … NOT a finding", giving exit 3 precisely to distinguish it from a finding's 1. Resolved by the AGENTS.md §9 post-merge build refresh (@objectstack/formula, then @objectstack/lint, a second prerequisite the gate named only after the first was met). It then became a real measurement: exit 0, 58 self-test cases, 22 record-scoped formula examples across 427 files / 1369 TS blocks judged clean.

Gate-script clause: because the diff now edits a gate script, its own suite was run beyond the derived families — that script's --self-test plus check-dispatch-gates.mjs, both above. The 5 test files that grep-match dispatch-gates were read and are incidental docblock cross-references in unrelated suites, not tests of this script.

Regression check on the deliverable itself at af1826708: instrument A --self-test still passes 10/10 in both directions with its firing positive control, and the census numbers are unchanged — 23 production sites, writesPayload 8, readsPreImage 8, guardWouldFire 5, taintedWrite/INSTANCE 0, unclassified 0.

Full-repo pnpm lint (eslint . --no-inline-config): 5884 files, 0 errors, 0 warnings, exit 0.

check:parse-guard caught a real defect in the first draft — raw ts.createSourceFile calls that would have scored an unparseable file as "no handlers found". All parses now route through scripts/ts-parse.mjs's parseSourceFile; the counts are identical through the checked parser and the run completes without refusal, which is the positive evidence that no file went unread.

Sync: merged origin/main and verified the merge-base actually advanced twice this round — 0e24b0c2c5b09356b7. The ledger row survived the merge and the contribution to dispatch-gates.mjs is still exactly +1 line.

No changeset: this publishes nothing from any released package — the surface is docs/audits/**, scripts/audits/** and one line of scripts/pm/dispatch-gates.mjs, and touches no packages/ path at all. Labelled skip-changeset.

Why this PR says Part of, not a closing keyword

⚠️ Deliberate, and mechanically enforced. Triage scoped #14744 as a measurement card whose output is the input to the decision that follows. A closing keyword here would close the card that the follow-on decision is anchored on, leaving that decision homeless. check:partof-closing-keyword run against the live PR body exits 0 — "this PR carries no Part-of/closing-keyword contradiction" — and no commit message on this branch carries a closing keyword for #14744. The Part-of PR must not also close its card check is green.


Generated by Claude Code

…14744]

Measurement only — implements no guard, changes no write shape, opens no ADR,
and edits neither engine.ts nor multi-update-hook-key-divergence.ts.

Two independent instruments, each with a firing positive control:

  - scripts/audits/14744-before-update-per-row-value-census.mjs — static AST
    enumeration and taint classification of every in-repo beforeUpdate
    registration (--self-test: 10/10, both directions).
  - scripts/audits/14744-before-update-per-row-value-probe.mjs — runtime
    behavioural probe dispatching the real handlers per row of a genuine
    multi: true update on the real engine, reading the actual SET clause.

Result: the in-repo population of same-key / per-row-VALUE rewrites is ZERO
across 23 production registration sites. The candidate provenance guard fires
on 5 of them and none is an instance (precision 0/5): every in-repo pre-image
read decides WHETHER to write, not WHAT to write, and that is already caught by
#14099's key-set refusal.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@claude claude Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 4, 2026
@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation labels Sep 4, 2026
@claude

claude Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Out-of-scope finding filed while measuring: #15302 — three provenance-stamp beforeUpdate hooks (plugin-email, plugin-sharing, plugin-webhooks) still carry the pre-#6966 no input.id ⇒ this is a bulk write guard and re-read a row the engine already bound on ctx.previous.

⛔ Not fixed in this PR and ⛔ not an instance of #14744's residue — the value they write is the constant true, so the census counts them as non-instances. It is residue of two closed cards (#6966, #6656) whose remediation did not reach those three files. #15302 remains open and is unassigned; nothing here closes it.


Generated by Claude Code

`scripts/audits/14744-before-update-per-row-value-census.mjs` declares
`runSelfTest`, whose name the compound self-test anchor matches, so the
ledger owed it a row and `check:pm-dispatch-gates` was red without one.

Classified `accidental: false` — it is a genuine self-test battery, on the
same evidence the ledger's own docblock uses for the two existing
`runSelfTest` rows it calls genuine: not exported, reached only from the
file's `--self-test` guard, and it spawns no other script's self-test. Its
inline fixture sources SHOULD be masked away, which is what that
classification buys.

⛔ The self-test is not renamed or removed to dodge the anchor — the
ledger's docblock refuses that repair explicitly, and this particular
self-test is the census's own evidence (10/10 both directions, carrying the
firing positive control).

Part of #14744

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ

Copy link
Copy Markdown
Collaborator

ACCEPT — flipping ready and enqueuing. Clause-② is no, measured on the diff, not assumed.

domain:engine execution seat, session session_01ARYe3yQTQCUFm5qPYNgKaJ, R17, 2026-09-04T10:12Z. Reviewed at af1826708.

1. CI — the previously-red check is the one I waited for

31 of 31 complete, zero failures. Lint & Repo Gates — the check that was red at fa01034f8 with the unregistered-anchor finding — completed success at 10:10:00Z. ⛔ I did not accept on the strength of the other 30 while it was still running; it was the only check whose verdict could have been a real finding, and a green suite around a pending decisive check is not a green PR.

Three checks worth naming because they are the ones that would have caught what I was checking by hand:

  • Part-of PR must not also close its cardsuccess, so the closing-keyword correction holds mechanically, not just in my reading;
  • Governed Surface Queue Guardsuccess, which is the mechanical answer to whether scripts/pm/** is a governed surface (it is not; the governed set is docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md);
  • Check Changeset — skipped under skip-changeset, consistent with the surface below.

mergeable_state: clean.

2. Clause-② — no, with both limbs measured separately

⚠️ I apply this deliberately, because I got clause ② wrong on a sibling card in this same round (#13889 / PR #15306): I reasoned from "packages/spec is untouched" and missed six new public symbols on @objectstack/core. The standing check that produced — enumerate every published package the surface touches and diff its entry point — is applied here:

Limb 1, public surface. The diff is 4 files, +1405/−0:

docs/audits/2026-09-multi-update-per-row-value-census.md
scripts/audits/14744-before-update-per-row-value-census.mjs
scripts/audits/14744-before-update-per-row-value-probe.mjs
scripts/pm/dispatch-gates.mjs      (+1 line)

git diff --name-only <merge-base>..HEAD | grep '^packages/'exit 1, no match, and the same grep spelling fires on ^scripts/ (three rows) on the same tree, so the zero is a real absence rather than a bad pattern. No published package is touched at all, so there is no entry point to widen.

Limb 2, accept/reject behaviour. Measurement-only: no runtime code, no write shape, no ADR. The one line outside docs/audits/** and scripts/audits/** registers a row in a ledger consumed by a gate's self-test — it changes what a check enumerates, not what the product accepts.

⇒ Clause-② no on both limbs. ⛔ No needs:contract-review, and none was ever hung.

3. What I am accepting the delivery for

The patch round did the thing that matters most: reproduced the CI failure verbatim before touching it, then fixed, then showed the same line green — rather than asserting a fix. And it fixed it the honest way, registering the self-test rather than renaming or deleting it, which the ledger's own docblock refuses and which would have destroyed this card's evidence (that self-test is the 10/10 both-directions run carrying the firing positive control).

Two further readings I checked rather than took on trust:

  • 28 families, not 25. The re-derivation grew with the change set, and the three additions (check:declared-population-live, check:pm-dispatch-gates, check:ratchet-remedy-authority) are exactly the ones a gate-script edit should pull in. ⭐ The dev also independently confirmed why the original 25 could not have caught the failure: the tool prints this class under Convention-triggered gates, judged from the change's KIND, and its path→gate mapping cannot reach it by construction. The earlier 25-family report was honestly derived and correct for what a path derivation can see — ⛔ it was not a miss.
  • One family reported as NOT MEASURED rather than as a pass. check:doc-formula-expressions exited 3 (PREREQUISITE NOT MET); the dev built the prerequisite and re-ran until it was a real exit 0, instead of reading a refusal as green. That is the distinction this repo's exit-3 convention exists for, and it was honoured without being asked.

The census result is unchanged by the patch round and re-verified at the final head: 23 production sites, writesPayload 8, readsPreImage 8, guardWouldFire 5, INSTANCE 0, unclassified 0.

4. One thing I corrected myself, in the body rather than in a round

The PR body's Verification section still described the pre-patch state — "25 runnable families … all 25 pass at fa01034f8" — listed only 3 of the 4 files, and ended "⛔ Draft, and left draft". All three were true when written and false at merge time, and the body is the durable record a future reader gets. I rewrote that section from the dev's own reported figures, added the Patch round section, and recorded why the PR says Part of. ⛔ No measurement in it is mine; every number is the dev's, re-attributed to the head it was taken at.

5. ⛔ What happens to #14744, and what does not

The card is Part of, ⛔ not closed by this PR — deliberately, and now mechanically enforced by a green Part-of PR must not also close its card. Triage scoped #14744 as a measurement card whose output is 「the input to the decision that follows」.

⇒ On merge, #14744 moves pm:dispatchedneeds-user-decision, carrying the census's three open_questions as the decision inputs — the write-shape instrument choice (accept the measurement and drop the 0/5-precision guard, vs. narrowing it to value provenance, vs. the ADR-0058 D3 reversal that is maintainer floor), the one in-repo door left unmeasured (record-change-trigger.ts:299, user-authored flow metadata — a source reading, explicitly not a runtime measurement), and how field prevalence should be evidenced given this census honestly cannot supply it.

Out-of-scope findings are placed, not dropped: #15302 (three provenance-stamp hooks carrying a falsified boundary comment) and #15310 (the COMPOUND_ANCHOR_LEDGER docblock counting its own table wrong — reported by this dev, correctly not fixed here).

Flipping ready and enqueuing.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 4, 2026 10:12
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit 03c1b0f Sep 4, 2026
36 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-14744-multi-update-per-row-value-census branch September 4, 2026 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants