Skip to content

After #6656 retires captureBefore, the ctx.__previous stash has no producer left — a dead fallback limb in trigger-record-change kept alive by its own fixture, plus four stale comments #6978

Description

@os-zhuang

Observation-class finding from the #6656 dev seat (PR #6977). Nothing a user hits today — dormant code and stale prose — so it carries finding and no pm:queue, per the filing discipline. Filed unassigned; routing left to triage.

Blocked-by: #6656 (the retirement is what creates both items; neither is actionable before it lands).

Fact

plugin-audit's captureBefore was the only producer of ctx.__previous in the repo. PR #6977 retires it. Measured on that branch after the change:

$ grep -rn "__previous\s*=" --include=*.ts packages apps | grep -v node_modules
packages/triggers/trigger-record-change/src/record-change-trigger.test.ts:362:
    (ctx as unknown as { __previous: Record[string, unknown] }).__previous = { status: 'old' };

$ grep -rn "\.__previous\b" --include=*.ts packages apps | grep -v node_modules | grep -v "__previous\s*="
packages/triggers/trigger-record-change/src/record-change-trigger.ts:299:
    ((ctx as unknown as { __previous?: Record[string, unknown] }).__previous ?? undefined);

(Generic brackets softened above so the issue sanitizer does not eat them.)

So the one surviving producer is a test fixture that writes the key itself, and the one surviving consumer is the fallback limb that fixture exists to exercise.

Item 1 — the dead limb (the only code item)

packages/triggers/trigger-record-change/src/record-change-trigger.ts:299:

const previous =
    (ctx.previous as Record[string, unknown] | undefined) ??
    ((ctx as unknown as { __previous?: Record[string, unknown] }).__previous ?? undefined);

Canonical key first, so this is the inverted reverse-verification family, not the usual one: deleting the limb changes nothing that any engine path can produce, and every existing test stays green — because record-change-trigger.test.ts:352 ("reads the __previous stash when ctx.previous is absent") synthesises the key in the test body. That is the #4984 shape exactly: a fixture spelling a key no producer emits, keeping a dead limb looking live and making the limb's deletion invisible to every gate.

Suggested disposition under ADR-0049 enforce-or-remove: delete the limb and the fixture that feeds it. The doc comment two lines up ("with the __previous stash audit also uses as a fallback") goes with them.

Item 2 — four comments that assert something no longer true

None of these change behaviour; all four will read as current fact to the next author, which is the mechanism Prime Directive #13 names ("the file being edited never named the ADRs that governed it") and that #6966 is a live instance of.

file what it says why it is stale after #6656
packages/objectql/src/engine.ts:7842 enumerates plugin-audit captureBefore / writeAudit before+afterDelete among the hooks holding the delete pre-image gate open plugin-audit registers no beforeDelete hook now; only writeAudit on afterDelete holds that term
packages/objectql/src/plugin.ts:918 "captureBefore's now-redundant read is the identity lane's follow-up" forward reference to #6656 itself; becomes past tense
packages/metadata/src/metadata-manager.ts:311 "plugin-audit's captureBefore threads the transaction by hand for exactly this reason" — cited as evidence the single-connection-pool stall is current the cited example no longer exists. The hazard claim is independent and still stands on DatabaseLoader._find(); only the witness is gone, so this needs a replacement example rather than deletion of the policy
packages/spec/src/contracts/scoped-context.ts:64 "plugin-audit's persistAuditTrailRow and its captureBefore snapshot both reach api.sudo()" only the writer reaches sudo() now. The conclusion (sudo excluded on purpose) is unaffected

Why this was not fixed in PR #6977

Its scope is packages/plugins/plugin-audit, and two of the four files are in packages/objectql, which that card explicitly fences off (the engine half is landed and closed; sys_fetch_previous_delete is #5929, engine-core lane). Item 1 is in packages/triggers. Cross-lane by construction, hence a standalone card rather than a rider.


Generated by Claude Code

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions