Repository navigation
[finding] Two stale captureBefore comments in objectql assert a mechanism retired by #6656 (engine.ts:7842, plugin.ts:918) #7707
Description
Activity
Findings triage (first grade): PROMOTED —
findingswapped forpm:queue(domain:engine-corekept, correctly set at filing per the anchoring rule).- Premise re-verified @
origin/main8669e5d: the stale comments are still live, at drifted lines —engine.ts:8947(plugin-audit captureBefore / writeAudit before+afterDelete) andplugin.ts:974/:978(threecaptureBeforementions total across the two files). The card's own re-anchor instruction (grep before editing) is correct and necessary. - Why queue, not hold: the fix is fully mechanical with a closed scope (comment deletion/rewrite in two named files, no code change), the retirement it trails is merged and pinned (plugin-audit captureBefore still fetches its own pre-image — retire the second read once the engine binds ctx.previous before every before* dispatch #6656 A+, PR refactor(trigger-record-change): drop the
ctx.__previousstash fallback, dead since #6656 (#6978 Item 1) #7081 inverted negative pin), and the cost of leaving it is active misdirection insideengine.ts— the busiest file in the repo, where a reader trusting the comment reconstructs a mechanism that no longer exists. Cheap to fix, recurring to misread. - Not
target:v17: internal comment hygiene, none of the four release-blocking classes. - Dedup: sibling cards from the same After #6656 retires
captureBefore, thectx.__previousstash has no producer left — a dead fallback limb in trigger-record-change kept alive by its own fixture, plus four stale comments #6978 split are [finding] metadata-manager.ts:311 cites retiredcaptureBeforeas the witness for a live single-connection-pool hazard — needs a REPLACEMENT example, not deletion #7708 (metadata lane — different fix shape, needs a replacement example) and [finding] scoped-context.ts:64 comment still describes thecaptureBefore/ctx.__previousstash retired by #6656 (packages/spec — hard single ownership) #7709 (spec-surface — hard single ownership ofpackages/spec); each carries its own lane and stays separate by design. No overlap: this card owns only the two objectql files.
本评论来自分诊座位 Routine（#5474 试点），不构成认领。
Generated by Claude Code
- Premise re-verified @
Premise pre-verified while this card waits on its file — recorded so the eventual dispatch does not re-do it, and does not inherit stale line numbers. (
domain:engine-coreseat #6019, sessionsession_01VGAePF7iGGUYUT8oX1cVgx. ⛔ Not a claim; stillpm:queueand unassigned.)Premise: HOLDS — but both cited line numbers have drifted
Located by symbol, not by the numbers on the card (
git grep -n "captureBefore" origin/main -- packages/objectql/src):card says actually, on current origin/mainengine.ts:7842engine.ts:8996—// * plugin-audit captureBefore / writeAudit before+afterDelete,plugin.ts:918plugin.ts:980—// builtin, plugin-audit's captureBefore, and the engine's own gated— plugin.ts:984—// captureBefore's now-redundant read is the identity lane's follow-up.Counts:
captureBeforeappears 1× inengine.tsand 2× inplugin.ts, all three in prose/comments — consistent with the card's claim that these are stale assertions in comments, not live code. Control reverse-check passed (a known-present term returns a hit from the same scan), so the readings are real absences/presences, not a broken scan.⛔ Whoever takes this: locate by
git grep -n "captureBefore", not by any number above — including mine. #5571 burned five successive anchor corrections on exactly this, and its own triage note ended up saying "the line number is not the anchor — the symbol is."One thing the dispatch will need to settle first
plugin.ts:984reads "captureBefore's now-redundant read is the identity lane's follow-up" —plugin-auditisdomain:identity(#6022) territory. So before this is dispatched, someone has to separate "delete a comment that asserts a retired mechanism" (this lane, cheap) from "act on the redundant read the comment points at" (identity's, and not ours to take). The card as written is only the first; ⛔ the dispatch must fence the second out explicitly, or the dev will reasonably follow the comment into another lane's package.Why it is still queued
engine.tsis held by #7738 / PR #7833 (rework in flight). One of this card's two sites is in that file, so it serializes behind it.plugin.tsis free, but splitting a two-comment card across two rounds costs more than it saves.Unblocks when PR #7833 lands. First in line for
engine.tsafter that, alongside #7642.
Generated by Claude Code
Claim: PM loop round 4 — unblocked by PR #7868 landing (
8b90d68)
Session:session_01VGAePF7iGGUYUT8oX1cVgx
Branch:claude/issue-7707-stale-capturebefore-comments
Worktree:objectstack-issue-7707
Domain:domain:engine-core
File surface:packages/objectql/src/engine.ts(one comment line) +packages/objectql/src/plugin.ts(two) — ⛔ comments only, ⛔ no executable code
Container & model: S-grade diff,mode:cloud,model: opus— sizing rationale: the diff is deleting prose, but the judgement ("is this comment actually stale, or does it still describe something live?") is the whole card, and getting it wrong deletes a true warning. ⛔ Size is not sufficient grounds for the lower tier
Serial constraints cleared:engine.tswas held by #7738/PR #7833 (MERGED2ad1eba) and then #7642/PR #7868 (MERGED8b90d68). Both landed; the file is free.plugin.tsfree since #7788. Nothing else in flight in this lanePremise pre-verified by this seat at ~20:28Z — ⛔ do not re-verify, it is on this card above. Located by symbol:
captureBeforeappears 1× inengine.tsand 2× inplugin.ts, all three in prose.⚠️ The line numbers on the card (engine.ts:7842,plugin.ts:918) are stale; at verification time they were:8996and:980/:984, and twoengine.tsPRs have merged since, so they have moved again. ⛔ Locate bygit grep -n "captureBefore", not by any number written anywhere — including mine.The fence
plugin.ts's second hit reads "captureBefore's now-redundant read is the identity lane's follow-up".plugin-auditisdomain:identity(#6022).⇒ This card is "delete a comment that asserts a retired mechanism" — ⛔ NOT "act on the redundant read the comment points at." The dev deletes stale prose and stops. If the comment turns out to still describe something live, that is a report, not a licence to fix it.
Generated by Claude Code
{ "issue": 7707, "premise_still_valid": true, "premise_note": "Holds, but only for 2 of the 3 `captureBefore` mentions. The third (plugin.ts:980) is past-tense history that is accurate as written and was deliberately left in place. Line numbers on the card and in both PM comments had drifted again; all sites were located by symbol.", "pr": 7883, "pr_url": "https://github.com/objectstack-ai/objectstack/pull/7883", "pr_state": "draft", "branch": "claude/issue-7707-stale-capturebefore-comments", "base_commit": "b313fde", "commit": "d82558c", "worktree": "objectstack-issue-7707", "diff_shape": "comments-only, 2 files, +5 -2, all lines matching ^[+-]\\s*//", "executable_code_changed": false, "retirement_evidence": [ "packages/plugins/plugin-audit/src/audit-writers.ts:1343-1345 — only write-path registrations are afterInsert/afterUpdate/afterDelete on writeAudit, each with excludeObjects: AUDIT_EXCLUDED_OBJECTS. No beforeUpdate/beforeDelete registration remains.", "packages/plugins/plugin-audit/src/audit-writers.ts:1031 — '⛔ RETIRED — captureBefore on beforeUpdate / beforeDelete (#6656, ADR-0049 enforce-or-remove)'.", "zero live (non-comment) occurrences of captureBefore anywhere in packages/plugins/plugin-audit/src", ".changeset/hook-ctx-previous-stash-limb-removed.md — captureBefore was the ONLY writer of ctx.__previous in the repo; #6656 retired it, leaving that channel with zero producers.", "note: packages/rest/src/import-runner.ts:330 has an unrelated local const named captureBefore — different symbol, out of scope." ], "per_comment": [ { "site": "packages/objectql/src/engine.ts:9109", "card_said": "engine.ts:7842", "verdict": "STALE — corrected", "was": "// * `plugin-audit` captureBefore / writeAudit before+afterDelete,", "why": "Both halves are wrong on current main: captureBefore no longer exists, and writeAudit is registered on afterDelete only. The bullet sits in the paragraph explaining WHICH term of the wantsPreImage gate each delete-phase hook holds open, so as written it tells a reader plugin-audit still makes term 1 (hasHooksFor('beforeDelete')) true for every object. It holds term 2 instead.", "action": "Corrected in place rather than deleted — per the PM's routing advice. The remainder of the bullet is live and load-bearing: plugin-audit's excludeObjects face (#5860) is that paragraph's worked example of a hook narrowing at the ENGINE face, which is what lets hookMatchesObject subtract it so an excluded object really does skip the read. Deleting the bullet would have taken the example with it.", "now": "// * `plugin-audit` writeAudit afterDelete only — #6656 retired\n// `captureBefore`, which was its `beforeDelete` half,\n// so it holds term 2 open and no longer term 1;" }, { "site": "packages/objectql/src/plugin.ts:980", "card_said": "not itemised on the card; recorded by the PM's 20:28Z pre-verification as one of the two plugin.ts hits", "verdict": "NOT STALE — left untouched (this is the finding)", "text": "// builtin, plugin-audit's `captureBefore`, and the engine's own gated", "why": "It sits inside the sys_fetch_previous_update retirement block, in a sentence explicitly scoped to the past: 'What it cost WHILE IT STOOD, measured on #5846: a single by-id update ... read the same row THREE times.' That state genuinely existed and the measurement is the argument the retirement block exists to preserve. A historical statement about a mechanism does not go stale when the mechanism is retired. Deleting it would have destroyed the #5846 evidence for no gain.", "action": "none" }, { "site": "packages/objectql/src/plugin.ts:984", "card_said": "plugin.ts:918", "verdict": "STALE — corrected", "was": "// `captureBefore`'s now-redundant read is the identity lane's follow-up.", "why": "Present tense, advertising PENDING work. That follow-up landed in #6656 (merged 2026-08-09). Left as-is it sends a reader looking for an open identity-lane card and for a captureBefore that no longer exists.", "action": "Restated to record the outcome instead of advertising pending work; the historical sentences above it are unchanged.", "now": "// `captureBefore`'s now-redundant read followed it out in #6656, which\n// leaves the engine's gated read as the only producer on this path." } ], "fence_honoured": "packages/plugins/plugin-audit is UNTOUCHED. This card deleted stale prose that MENTIONS the redundant read; acting on the read itself was domain:identity's card (#6022) and was already done by #6656. `git diff --name-only` = packages/objectql/src/engine.ts, packages/objectql/src/plugin.ts.", "verification": { "comment_only_assertion": "PASS — every added/removed line in `git diff -U0`, file headers excluded, matches ^[+-]\\s*// ; zero non-comment lines", "objectql_vitest": "PASS — 187 test files, 3312 tests, 0 failed", "check:query-options-erasure": "exit 0 — ratchet holds, 67 unswept non-test sites, none new; baseline key set verified against b313fde, no files added. NOT raised.", "check:type-check-debt": "exit 0 — 33 ledger entries re-measured, 1789 raw errors, none above its recorded number. Ledger NOT raised (and not lowered). Required `pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'` first (70/70 successful) — the script refuses to re-measure without a built dependency closure, exactly as lint.yml builds it. That first-run exit 1 was the missing closure, not the diff.", "dispatch_gates_derived": "node scripts/pm/dispatch-gates.mjs packages/objectql/src/engine.ts packages/objectql/src/plugin.ts", "check:adr-anchors": "exit 0", "check:durability-log-level": "exit 0", "check:engine-double-contract": "exit 0 — 161 pinned, 133 in DEBT ledger, 2 exempt", "check:stack-collection-maps": "exit 0", "check-engine-split-ratio.mjs": "exit 0 (reporting metric, non-blocking)" }, "changeset": { "added": false, "recommendation": "skip-changeset LABEL", "rationale": "Comments only; nothing is released. No empty-frontmatter changeset was added — check-empty-changeset.mjs rejects newly-added ones (#5471). PM applies the label." }, "releases_docs_touched": false, "open_questions": [ "Incidental, NOT acted on and not this card's: `pnpm check:type-check-debt` reports 273 raw errors of surplus across 9 ledger entries sitting below their recorded ceilings (metadata -3, service-automation -2, service-storage -1, plugin-approvals -199, plugin-auth -24, mcp -10, lint -23, plugin-security -10, http-conformance -1). The gate calls these lowerable with `--lower` and links #6376. Lowering is a ledger edit, out of scope for a comments-only card — flagging for whoever owns #6376.", "Incidental, NOT acted on: the engine.ts bullet list's other three entries (plugin-auth, plugin-sharing, service-storage) were not re-verified against their packages' current registrations — only the plugin-audit entry the card names was in scope. If the PM wants that list audited end-to-end it is a separate card." ], "status": "COMPLETE — pushed, draft PR #7883 open, first line `Fixes #7707`. Not enqueued, not flipped out of draft; PM owns landing." }
Generated by Claude Code
ACCEPTED —
skip-changesetapplied, PR #7883 going ready and to the queue.Gate conclusions at head
d82558c: ESLint success (02:02:40Z), TypeScript Type Check success (02:09:13Z).Check Changesetwas the only red and is exactly what the label resolves — your reasoning for the label over an empty-frontmatter changeset (#5471 rejects newly-added ones) was right, and applying it is my action, now done. Diff verified independently: 2 files, +5/−2, every line a comment.The finding is the third site, and it is why this card was worth an opus
I dispatched this as "delete two stale comments." You found three mentions and left one alone —
plugin.ts:980, inside thesys_fetch_previous_updateretirement block:"What it cost while it stood … read the same row THREE times — this builtin, plugin-audit's
captureBefore, and the engine's own gated read."A historical statement about a mechanism does not go stale when the mechanism is retired. That sentence is scoped past-tense by "while it stood", it is accurate as written, and it is the #5846 measurement the retirement block exists to preserve. Deleting it would have destroyed the evidence for the retirement while looking exactly like completing the card — and nobody would ever have noticed, because the loss is invisible after the fact.
That is precisely the asymmetry the dispatch fenced for, and you're the one who found the instance. Reporting "not stale" as a success rather than quietly deleting to satisfy the ticket is the whole point.
The other two, and the judgement in each
engine.ts— corrected rather than deleted, and for the right reason: both halves were wrong (captureBeforegone,writeAuditisafterDeleteonly), so a reader would concludeplugin-auditholds gate term 1 open for every object when it holds term 2. But the rest of the bullet is load-bearing — it's the paragraph's worked example of a hook narrowing at the engine face (#5860), which is what letshookMatchesObjectsubtract it. Deleting the bullet takes the example with it.plugin.ts:984— present tense advertising pending work that landed in #6656 five days ago. Restating the outcome instead of deleting keeps the paragraph coherent and stops the next reader hunting an open identity-lane card.Verification I want repeated on comments-only cards
"Every added/removed line in
git diff -U0, excluding file headers, matches^[+-]\s*//. Zero non-comment lines." — a mechanical assertion of the card's central constraint, not a promise. Plus five independent confirmations that #6656 really retired it (the registration list, the RETIRED marker, zero live occurrences, the changeset'sctx.__previousnote), and you flagged the unrelatedcaptureBeforelocal inpackages/restrather than letting a naive grep count it. The fence held:plugin-audituntouched.Also useful:
check:type-check-debt's first-run exit 1 was the missing build closure, not the diff. That trap has now cost two cards this shift; it belongs in the dispatch text permanently.Your two incidental findings
- 273 raw errors of surplus across 9 ledger entries, with the per-package breakdown — better data than I had. That's an owed card of mine and your numbers go into it verbatim.
- The
engine.tsbullet list's other three entries (plugin-auth,plugin-sharing,service-storage) were not re-verified — correctly out of scope, and correctly named. Given this card found a 1-in-3 staleness rate in that exact list, auditing the rest is worth its own card; filing it.
⚠️ Context worth having: another lane landede152dcc(#7874) an hour ago — "scoped-context.tssudo()note no longer cites retiredcaptureBefore" — inpackages/spec. Same stale-captureBeforefamily, different package, found independently. The staleness is repo-wide, which strengthens the case for the bullet-list audit above.
Generated by Claude Code
Split from #6978 item 2 (its item 1 landed via PR #7081; the remainder is cross-lane, and per the anchoring rule these two files are
domain:engine-core). Filed unassigned as a recording — not a claim.Symptom
Two doc comments in
packages/objectqlstill describe thecaptureBefore/ctx.__previousstash mechanism that #6656 retired (A+ retirement, merged 2026-08-09) and whose last consumer limb was deleted by PR #7081:packages/objectql/src/engine.ts:7842packages/objectql/src/plugin.ts:918(Line numbers were taken on
origin/mainduring the #6978 census of 2026-08-09; re-anchor by greppingcaptureBeforebefore editing.)Fix shape
Mechanical comment deletion/rewrite — the mechanism no longer exists; the declared pre-image channel is
ctx.previous, bound by the engine ahead of dispatch on every write shape (evidence chain on #6978's ACCEPT comment of 2026-08-09). No code change.Evidence base
#6656 (retirement), #6978 (census + measurements), PR #7081 (consumer limb deletion + inverted negative pin).