Repository navigation
feat(plugin-audit): the platform stamps sys_comment is_edited / edited_at when a comment's body is edited - #23153
Conversation
…at on a body edit The comment beforeUpdate stamp sets is_edited and edited_at (server clock) when an update's body differs from the stored body, for every caller; both fields become readonly, so a client's value is hidden from the hooks and stripped on update, and the create hook replaces it on create. Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude <noreply@anthropic.com>
…nt fixture Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude <noreply@anthropic.com>
…narrowing) Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 1 package(s): 18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 4 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a2d600da442abbbd2d8aa91e6f22f656946492a2 && git checkout a2d600da442abbbd2d8aa91e6f22f656946492a2
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 121847a7d576f5e815d71ea7bdfcefd13cecdbb6 67900682e266fe9aa7f1834eea8c0a5aece2da8b && git checkout -B drift-repro 121847a7d576f5e815d71ea7bdfcefd13cecdbb6 && git merge --no-ff 67900682e266fe9aa7f1834eea8c0a5aece2da8b
node scripts/docs-audit/affected-docs.mjs --json 121847a7d576f5e815d71ea7bdfcefd13cecdbb6
|
Contract reviewServed-tier: ① Derived judgmentsInputs: card #23124 (body; ruling 6137155820; claim 6137424622; the dev's report 6138086399), PR #23153 (body, five-file list, net diff against The diff moves one published contract,
Nothing in the diff widens an accept set; no export, route or spec key moves. ② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
Fixes #23124
Clause-②: no (narrowing) — it narrows a published accept set: a client's write to
is_edited/edited_atno longer takes effect. Per the ruling, it owes one contract-review-tier review before the queue.What this does
Executes maintainer ruling A (recorded in comment 6137155820): the platform, not the console, marks an edited comment.
sys_commentgets a secondbeforeUpdatehandler,stampCommentBodyEditincomment-access-hooks.ts. It setsis_edited: trueandedited_at(server clock, ISO string) when the update'sbodydiffers from the stored body (ctx.previous.body). An update that does not namebody, or sends the stored text again beside newmentions(the console's shape), stamps nothing. It runs for every caller, system included, the way the engine'supdated_atstamp does. UnderpreserveAudit(historical import) a supplied value is kept, again asupdated_atdoes. It is registered on its own, not inside the authorization handler, because that handler returns early for system and context-less callers and declares the whole-operation dispatch.is_editedandedited_atdeclarereadonly: true.sys_objects to their own guards on create (staticReadonlyInsertSubject), so the comment create hook sets both fields to "not edited" when a client sends them. This works the same way as theauthor_idstamp next to it.sys_commentdocblock records the decision (ADR-0049, declared = enforced), in the same voice as its two earlier entries. The module header ofcomment-access-hooks.tspoints to the stamp.Mechanism hypotheses, measured
The rig: base
1ca7a805c, a real kernel with ObjectQL, AuditPlugin and the SQLite driver. The caller is the comment's author.H1 holds.
git grep -n "is_edited\|edited_at"outside the generated translation bundles returns 2 hits, the two declarations insys-comment.object.ts. A body edit storedis_edited: falseandedited_at: null. The author's edit already passes the existing authorizationbeforeUpdate.H2: what the engine does today, measured on the
created_atprecedent.{ body, created_at: forged }created_atunchangedonFieldsDroppedreported{ fields: [created_at], reason: readonly }. This is the REST PATCH'sdroppedFields, plus awarnlinestrictReadonlyWrites: trueERR_READONLY_FIELD_REJECTEDthrown. This is an in-process opt-in, and no transport passes it{ created_at: forged }{ is_edited: true, edited_at: forged }, before this changeChoice: (a) and (b) together. On update,
readonly: truegives the engine's strip-and-report, and the stamp's overwrite wins on an edit. On create, the comment create hook overwrites. There is nopackages/specchange.There is no loud refusal, for two reasons:
.objectui-sha(1fb1ced) and on objectui main (5d3f437) sendsis_edited: true, edited_at: CLIENT_CLOCKwith every edit (handleEditCommentinRecordDetailView.tsx). The ruling files the console's removal only once this release is installable, so a refusal would break comment editing in the shipped console for that whole interval.The strip is not silent at the REST door: it shows up in
droppedFieldsand in awarnline. The one silent case is a client value sent together with a body edit. There the stamp replaces it with the true value, the same shape ascreated_at/updated_at.H3: no bypass. A
multi: truepredicate update dispatchesbeforeUpdateonce per matched row, withinput.idandpreviousbound. Measured: 2 matched rows gave 2 dispatches, each carrying its own row's prior body. What that means for the stamp, all pinned:MULTI_UPDATE_HOOK_KEY_DIVERGENCE, 400, nothing written). One SET clause cannot mark some rows and not others.Other doors checked:
update.options.upsertis a retired tombstone.insert/insertManydispatchbeforeInsertonce per row.updateData/updateManyData, the import runner and the automation CRUD nodes all go throughengine.update.engine.execute(), a driver statement. It bypasses every hook and every strip for every object by design, no transport exposes it, and its only callers in the tree are migrations. No caller writessys_commentthrough it.So there is no fork.
Pins
comment-edit-stamp.integration.test.tshas 10 tests on the real engine:mentions-only edit does not stamp, with or without the unchanged body sent beside it.is_edited: true, edited_at: forgedstores "not edited".is_edited: false, edited_at: forgedis stampedtruewith the server clock.{ is_edited: true, edited_at: forged }and later{ is_edited: false, edited_at: null }change nothing. Each is reported as{ reason: readonly, fields: [is_edited, edited_at] }.mapDataErrorgives 400MULTI_UPDATE_HOOK_KEY_DIVERGENCE, the error'skeysare[edited_at, is_edited], and nothing is written.preserveAuditupdate keeps the mark it supplies.Run against the base's implementation (both source files restored from
1ca7a805c, test file unchanged):Tests 7 failed | 3 passed (10). The 3 that pass are 2, 7 and 10. They pass there by construction, because nothing was stamped and nothing was read-only, so they carry no weight alone. The no-stamp pins 2 and 7 are held by the ablations below.Ablations
Each ran through
scripts/ablation-replace.mjsin wrap mode, after the implementation was committed. The anchor hit was verified on disk, and each restore was proven: blob equal to HEAD andgit diff HEADempty.body-present guardan update that does not name body stamped the comment), pin 5an update re-sending the stored body stamped the comment), pin 7 (unchanged row … was stamped), pin 8 (a mixed predicate update was accepted)readonly: falsea client marked an unedited comment edited: expected { is_edited: true, …(1) } to deeply equal { is_edited: false, edited_at: null })ctx.submitted.* ?? stamp)the client cleared the mark on its own edit: expected false to be true)a client-supplied mark landed on create)A3's first attempt used a replacement that was a substring of its anchor. The tool refused it (the replacement count could not rise), so it never ran. It was re-run with
readonly: falseand went red as shown.Tests
All on HEAD
67900682e2:pnpm --filter @objectstack/plugin-audit test:Test Files 48 passed (48),Tests 771 passed (771).pnpm --filter @objectstack/plugin-audit typecheck: exit 0, includingcheck:test-typecheck: OK.tsc --listFilesovertsconfig.test.jsonlists both edited test files.pnpm --filter @objectstack/plugin-audit build: exit 0.Gates
The gate list comes from
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, run with no paths at HEAD67900682e2. It gives 67 commands, the same as the PM's list for tree1ca7a805c. All 67 were run at67900682e2:pnpm check:i18nfirst exited 3 (PREREQUISITE NOT MET: the CLI was not built). After the gate's own one-command prerequisite build it readscheck-i18n-bundles: OK (10 package(s) — all bundles in sync, no undeclared authoring keys), and plugin-audit's 7 bundles are in sync.pnpm check:dual-build-cjs-loadsis NOT MEASURED. It exited 3, PREREQUISITE NOT MET, because it reads every package's built output and 54 packages have nodist/in this worktree. It needs a whole-repopnpm build, so it is left to CI. plugin-audit's own build passescheck-dts-emitted, and its exports are unchanged.dispatch-gates --ran, with an exit code recorded for every command: 67 derived, 66 run, 1 NOT-MEASURED (derived from the recorded exit 3), 0 UNRUN.Verdict lines quoted from the gates:
check-adr-0087-registration:✓ 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition, reading[BREAKING+clause-②-narrowing] not-required (no-migration-prescription).check-changeset-no-major:✓ This diff introduces no major bump.check-empty-changeset:✓ No empty-frontmatter changeset introduced by this diff.check:nul-bytes:OK … no raw ASCII control bytes.check:test-source-alias:OK.Lint, narrowed and stated as a measurement:
pnpm exec eslint --no-inline-config --format jsonon the 4 changed.tsfiles: 4 files, 0 errors, 0 warnings.ESLint.isPathIgnoredreturns false for all 4).eslint.config.mjsnever enables type-aware linting (noparserOptions.project), so this diff cannot move the verdict of any file it does not touch.pnpm lintrun belongs to CI.Acceptance notes
audit-plugin.tsis untouched, and no translation bundle changed, since no label or description changed. Two edits to the existingcomment-access-hooks.test.tsare harness changes, not new pins. Its fake engine kept the last handler per event, so the stamp's registration would have replaced the authorization handler those tests drive. The harness now keeps the first handler per event. Its wiredsys_commentfixture now declares the two fields the stamp writes, because the engine refuses undeclared keys.Clause-②line, copied above as written, carries no direction arm, but its reasoning names a narrowing. The changeset declaresClause-②: no (narrowing)together with the BREAKING banner and an ADR-0087not-required (no-migration-prescription)marker, andcheck-adr-0087-registrationreads it as[BREAKING+clause-②-narrowing]. If line 2 of this body should carry the arm as well, the edit is to insert(narrowing)afterno.minor, under the launch-window convention for accept-set narrowings. The contract review judges the level.handleEditCommentbecomes inert: its values are stamped over on an edit and stripped otherwise.mapDataErrormaps aReadonlyFieldRejectedError(thestrictReadonlyWritesrefusal, which carries nostatus) to404 OBJECT_NOT_FOUND. This was measured in-process only. No transport passesstrictReadonlyWrites, so nothing public reaches it.Generated by Claude Code