Skip to content

fix(plugin-audit): a fired milestone activity row is withheld from a reader not served its watched fields - #22814

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-22786-withhold-milestone-row
Oct 11, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-22786-withhold-milestone-row

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #22786

Clause-②: no

A narrowing of what a reader is served; no accepted input changes and no public type moves (triage 6106418840, claim 6106790619).

What changed

packages/plugins/plugin-audit/src/activity-field-redaction.ts. The withheld-row pre-scan from #21388 now makes ONE judgement per row, isWithheldRow. A row is withheld when either arm holds:

Both arms share one pre-scan, one per-read served answer, and one WHERE on find / findOne / count / aggregate. A pre-scan failure denies the read, as before. There is no second judge.

  • One answer for both halves. The row is withheld exactly when the column redaction would drop the row's metadata.kind. Both judge a declared value through one helper, sourcesServed. A declaration with no readable list is judged by one per-read unknown-provenance answer, restrictedPerRead. That answer was lifted out of redactActivityRows, so the middleware hands the same answer to the pre-scan and to the redaction. computeWithheldUpdateFilter called without that answer judges such a row closed.
  • Route 2 is not ridden along. Route 2 adds the watched field to the summary's sources. Once the row is withheld from that reader, route 2 changes nothing at the served read, so it would only restate the same answer in a second place.
  • Out of the diff: no packages/spec, no new type value, no new column, and no export from the package entry.

The PM's mechanism assumptions, measured

  1. Holds. On 680a86b4c, isWithheldOnlyUpdate is at :271 and the pre-scan calls it at :321 with the reader's served set.
  2. Holds for every row the mirror writes from fix(plugin-audit): a fired milestone row carries metadata.kind 'milestone' (ADR-0052 §5), served to a reader served its watched fields #22783 on. matchMilestone returns every field a declared milestone watches, and the writer stamps that list beside kind: 'milestone'. So the list is the right input and is reused as is.
    • Rows written before fix(plugin-audit): a fired milestone row carries metadata.kind 'milestone' (ADR-0052 §5), served to a reader served its watched fields #22783 carry no metadata.kind, so nothing stored on them says a milestone fired. They are judged exactly as before this change: key-by-key narrowing plus the update arm. They age out with the object's retention, which is the precedent the module header already sets for the mirror's earlier shape.
    • The 'unknown' precedent applies only to a row that carries the kind but no readable list. The mirror never writes such a row. It is judged as the text precedent judges one: served only to a reader not restricted on the parent.
    • Whether earlier rows need more than this is raised with the dispatching seat. It is not decided here.
  3. Holds. A withheld row reaches the reader through no column at all. This is pinned on the list read scoped by object_name + record_id, on the by-id read, and on count, in the same harness fix(plugin-audit): a fired milestone row carries metadata.kind 'milestone' (ADR-0052 §5), served to a reader served its watched fields #22783's tests use.

Pins (activity-milestone-kind.integration.test.ts)

These run on a real engine, a real SQLite driver, the real CRUD mirror and the real AuditPlugin middleware chain, with the security service as the one stand-in:

  • A reader withheld a watched field gets neither milestone row: not on the list read and not by id.
  • CONTROL: a reader served every field gets each milestone row with metadata.kind, both by id and on the list.
  • CONTROL: a reader withheld only a field no milestone watches still gets the milestone rows with the kind. This is the existing test, unchanged.
  • CONTROL: a row no milestone fired is served to the withheld reader deep-equal to the row the control is served. That reader is served exactly the non-milestone rows.
  • count agrees with the rows served, for every reader.
  • Fail closed: when the pre-scan's system read fails, a non-system reader's list read is empty and its count is 0. The system read is unaffected, and the read recovers once the pre-scan works again.
  • Unit pins cover isWithheldMilestone, isWithheldRow and the milestone arm of computeWithheldUpdateFilter, including the unknown-provenance and no-answer cases.

Two existing pins from #22783 asserted that the withheld reader was served the milestone row without its kind. They now assert the row is withheld, which is the behaviour this card changes. The key-for-key pin drops that reader's row.

Verification

Every reading below was taken at aa9b481c1, the head this PR opens at. That head has origin/main 179f7bf6c merged in: three docs-only commits, none touching a file this diff touches. Each command ran through scripts/pm/os-verify-lock.sh, with the exit code captured before any pipe.

  • Build: pnpm exec turbo run build over @objectstack/plugin-audit... and the check:i18n closure exited 0.
  • Tests: pnpm --filter @objectstack/plugin-audit exec vitest run --maxWorkers=2 passed 46 files and 738 tests.
  • Typecheck: pnpm --filter @objectstack/plugin-audit typecheck exited 0. That covers the source, the scripts and the test layer (check:test-typecheck: OK, 0 debt entries).
  • Dogfood: pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2 test/activity-field-values.dogfood.test.ts passed 10 of 10. It boots for real, with SecurityPlugin and REST. It is not owed, because no public surface moved. It was run as a control that a milestone row reaches readers who are served its watched field.
  • Gates: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 65 commands at this head. All 65 ran and each exited 0, including check:i18n and check:dual-build-cjs-loads with their prerequisites built. --ran reconciliation: 65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN.
  • Lint (a narrowed run, declared with its three proofs):
    • Population: eslint ran on the diff's two TypeScript files with --no-inline-config --format json. The diff's third file, the changeset .md, is answered by eslint itself with "File ignored because no matching configuration was supplied".
    • Count: the JSON output counts 2 files linted, 0 errors and 0 warnings.
    • Invariance: eslint.config.mjs enables no type-aware linting. --print-config on the source file resolves no parserOptions.project and no projectService. The only files the config reads are two baseline JSONs that this diff does not touch. So no untouched file's verdict can move.
  • Ablations (one-off, not left as tests): run from the committed state 76f2e23b9 through scripts/ablation-replace.mjs. In each run the anchor hit exactly once, and the blob was shown to change on disk and then shown restored to the HEAD blob with git diff HEAD empty. The subject resolves through src/ (this package's own tests import it relatively), so no dist/ preflight applies.
    1. Milestone arm dropped from isWithheldRow: 7 red and 15 green in the milestone file. The red ones are the four withheld-reader served-read pins (list, by-id, served set, count) and the arm's three unit pins. Every control stayed green.
    2. Pre-scan deny removed from the middleware's catch: exactly one red, the fail-closed pin (1 of 22).

CI was not awaited.

Acceptance notes


Generated by Claude Code

…der not served its watched fields

The withheld-row pre-scan (activity-field-redaction.ts) now makes one
judgement per row: an update whose every recorded change is withheld, or a
fired milestone whose declared watched fields are not all served. Same
pre-scan, same served answer, same WHERE on find/findOne/count/aggregate.
A declaration with no readable list is judged by the unknown-provenance
answer, now shared by the row rule and the column redaction.

Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp
Co-authored-by: Claude <noreply@anthropic.com>
Comment-only: the one withheld-row judgement's doc names what both arms
share, and a test comment is kept neutral.

Claude-Session: https://claude.ai/code/session_01CBAfsWMSfM3EToQGVStEcp
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-audit, touching 9 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/system-context.mdx (via installActivityFieldRedaction (symbol, a top-level function))
What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 23419bafb742fc13fa3c22b446263bbd6041c78e → packageMentionDocs.

Which tree this was computed on

This run read content/docs from c4304b756d11a72c702857e67a7709506e415b17 — the merge of head aa9b481c192024e31f1e5a23fae2e9c21e4db35d into base 23419bafb742fc13fa3c22b446263bbd6041c78e, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c4304b756d11a72c702857e67a7709506e415b17 && git checkout c4304b756d11a72c702857e67a7709506e415b17
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 23419bafb742fc13fa3c22b446263bbd6041c78e aa9b481c192024e31f1e5a23fae2e9c21e4db35d && git checkout -B drift-repro 23419bafb742fc13fa3c22b446263bbd6041c78e && git merge --no-ff aa9b481c192024e31f1e5a23fae2e9c21e4db35d

node scripts/docs-audit/affected-docs.mjs --json 23419bafb742fc13fa3c22b446263bbd6041c78e

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 23419bafb742fc13fa3c22b446263bbd6041c78e → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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/m tests tooling

Projects

None yet

2 participants