Skip to content

docs(plugin-audit): restore the published README's services.audit pointer, in a form published readers can follow - #9636

Merged
os-steve merged 1 commit into
mainfrom
claude/issue-9589-plugin-audit-see-also
Aug 18, 2026
Merged

docs(plugin-audit): restore the published README's services.audit pointer, in a form published readers can follow#9636
os-steve merged 1 commit into
mainfrom
claude/issue-9589-plugin-audit-see-also

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9589

Verdict: restore — but not the line that was removed

The card asked for a judgment, not a revert. Both conditions the card set are met, so the link goes back; the line that carries it is new, because the removed one was wrong in two ways that outlived the reason it was dropped.

Condition 1 — the page's defect is gone (read as it stands on main)

Verified against content/docs/kernel/runtime-services/audit-service.mdx at 14935ab71, not against the title of the PR that rewrote it:

  • Canonical source line 9: packages/plugins/plugin-audit/src/auth-event-audit.ts.
  • Method line 24: services.audit.recordAuthEvent(event: { ... }).
  • Action union line 25: action: 'login' | 'logout'.
  • The settings sink still appears, but only under an explicit "Not this: the settings audit sink" heading (line 84) which states it is never registered as or resolved from this slot, and that getService('audit').record({ ... }) fails with a TypeError. That is disambiguation, which is the opposite of the defect PR docs(plugin-audit): the published README stops documenting an API, a row shape and an action vocabulary that do not exist (#9517) #9531 measured — nothing on the page presents record() / 'set' | 'reset' as the audit slot.

Condition 2 — the page carries what the README deliberately does not

The card named three candidates and said a link is worth restoring only if at least one holds. All three do. The README's own "The audit service slot" section is lines 291-304 of packages/plugins/plugin-audit/README.md.

CandidateOn the pageIn the README
Error posture"Typical Errors": recordAuthEvent never throws; a failed insert is error once per process then debug; the row is lost and nothing retries; silent no-op when no engine resolves or userId is absentAbsent. The README's two failure postures (lines 217-224) belong to the record-view batcher — a different code path — and its slot section states no posture at all
organizationId visibilityLine 45: it stamps the row's tenant columns, and an unstamped row is one non-administrator members can never seeThe README's tenant note (lines 231-234) is about read rows written by the record-view writer. The slot section never enumerates the event's fields
Settings-sink disambiguationThe "Not this" section, with the TypeError consequenceThe README names config_change as written by service-settings, but never distinguishes SettingsAuditSink.record() from the slot

Plus the whole event shape — userId must be a real sys_user id, sessionId lands on record_id with object_name fixed to sys_session, context is serialized into metadata. The README's slot section stops at interface AuthEventAuditSink and the closed action union, so the page is the only place a reader learns what to put in the event. The link is not redundant.

Why the restored line is a different line

PR #9531 removed:

- [Audit Logging Best Practices](/content/docs/kernel/runtime-services/audit-service.mdx)

Wrong twice, independently of the page defect:

  1. The label describes a best-practices guide. The page is a service-slot reference and has never been anything else.
  2. The URL form resolves for neither audience a published README has. This README is in the package's files array with private unset, so it renders on npm and on GitHub, where a root-relative href resolves against npmjs.com / github.com. It is not a docs-site route either: apps/docs/lib/source.ts mounts loader({ baseUrl: '/docs' }) over content/docs, so the route is /docs/kernel/runtime-services/audit-service, and apps/docs/redirects.mjs has no /content source to rescue it.

The replacement uses the absolute form the repo already established, and its annotation states what the page adds — so the next author weighing the same omission can check the justification instead of reconstructing it.

One in-place fix, declared

packages/plugins/plugin-audit/README.md:343 carried the only site-root-relative docs link in the entire packages/ tree — ](/docs/permissions/permission-sets#access-depth...), added by the same PR #9531 — and it is converted to the same absolute form. Its target page and heading anchor both exist (content/docs/permissions/permission-sets.mdx:111, slug access-depth--readscope--writescope-adr-0057-d1); only the spelling was unfollowable off the docs site.

Boundary scan behind that call: grep -rn "](/docs" --include=README.md packages/ returned exactly one hit before this change and zero after. Same file, same defect class as the link being added, mechanically pinned by the convention below — leaving it would have shipped a README with one followable and one dead docs link, in a diff whose whole subject is which of the two forms is correct. The seven links in other packages are not touched here; they are filed as #9632.

PM hypotheses

H1 — does check:published-readme-symbols have anything to say about an outbound link? No. The expectation holds, and the gate's real name is check:published-readme-exports. There is no script or package.json entry named ...-symbols; PR #9546 added scripts/check-published-readme-exports.mjs (pnpm check:published-readme-exports, lint.yml:1723). It is explicitly "deliberately NOT a docs linter": it reads fenced code blocks only (typescript / ts / tsx / diff / untagged) and makes one claim per import line plus one per Name.member( call site, against the built .d.ts. Its only ]( occurrences are in its own header prose and self-test fixtures. Measured on this diff: all four changed lines (343, 395-397) sit outside every fenced block in the file — the six fences are at 24-26, 33-37, 43-49, 164-166, 296-300, 360-387. Also worth knowing for future cards: scripts/published-readme-exports.baseline.json names @objectstack/plugin-audit as the gate's deliberate negative control ("the PR that added this gate reports it clean"), so this file must stay clean rather than merely unbaselined.

H2 — is there any gate that would catch a published README linking to a page or anchor that does not exist? No, and the gap is real. Measured across every candidate: the lychee lane (check-links.yml) runs over exactly content/**/*.md, content/**/*.mdx, README.md, ARCHITECTURE.mdpackages/**/README.md is not in scope; check:doc-anchors takes its sources from content/** plus EXTRA_SOURCES = ['README.md', 'ARCHITECTURE.md'], the root README only; check:adr-links is scoped to docs/adr/; check:docs-redirects to apps/docs/redirects.mjs; and check:published-readme-exports per H1. A published README can link anywhere, in any spelling, and ship to npm green. Filed as #9632 with a contained three-assertion proposal and sizing — the load-bearing one needs no filesystem lookup and cannot false-positive, and the other two reuse the resolvers check-docs-redirects and check-doc-anchors already have rather than growing a third.

H3 — the card's shorthand was the wrong form, and the repo is inconsistent. Four spellings coexist in published READMEs: the absolute https://docs.objectstack.ai/docs/... (create-objectstack/README.md:90 and the blank template at 39, 70, 108 — the only form that works on all three surfaces, and the one adopted here); /content/docs/... (7 links, 5 packages — dead everywhere, filed as #9632); /docs/... (1 link, this file, converted above); and ../../../content/docs/... (2 links, service-knowledge and knowledge-ragflow — these resolve on GitHub and npm but land the reader on raw MDX source rather than the rendered page). So the card's /docs/kernel/runtime-services/audit-service was the second-rarest form in the tree and unfollowable off the docs site.

H4 — no other published README records this deliberate omission. Swept all package READMEs for omission markers (deliberately, intentionally, dropped, omitted, restore, once #, Blocked-by) and read every ## See Also section in the tree. The hits are all unrelated (service-realtime's unimplemented handleUpgrade, service-automation's deliberate non-duplication of the per-node reference, the spec liveness ledger's vocabulary). plugin-audit is the only README whose docs link was dropped for a wrong target page. The adjacent case — runtime-services/index.mdx pointing "Audit bridge" at the settings sink — is already open as #9588 and is a docs-site page, not a README, so it is not a duplicate of anything filed here.

Verification

Local gate union, run after the final commit, at a15cc19df:

check:nul-bytes OK (6176 text files; no raw ASCII control bytes)
check:published-readme-exports --self-test OK (both analysis directions pinned)
check-adr-0087-registration OK (1 non-breaking changeset seen)
check-empty-changeset OK (1 declaring changeset added)
check-changeset-no-major OK
check:test-source-alias OK (72 packages scanned)
check:type-source-resolution OK (76 packages scanned)
check-affected-docs OK (242 self-test cases)

Gate set derived from the actual changed paths with node scripts/pm/dispatch-gates.mjs packages/plugins/plugin-audit/README.md .changeset/plugin-audit-readme-audit-service-link.md, which added the four changeset families over the README-only derivation.

Two gates are reasoned rather than run, both build-dependent, both with the measurement behind the claim:

  • check:published-readme-exports (full run) hard-errors without a built dist/ for every workspace package any published README imports, i.e. a whole-workspace build; it runs in the CI job that has already built. Its --self-test passes, and the fence-range measurement in H1 shows this diff is outside its reading surface entirely.
  • check:i18n is convention-triggered here because plugin-audit owns scripts/i18n-extract.config.ts, and it needs @objectstack/cli built. That config's input set is three object modules — sys-audit-log.object.js, sys-activity.object.js, sys-comment.object.js — plus the generated bundles. A markdown prose edit changes none of them.

No test pins this README's content: grep -rn "README" --include="*.test.ts" across packages/ returns no plugin-audit hit, and the one structural reference (packages/spec/src/kernel/plugin-structure.test.ts) uses the literal string 'README.md' as a schema fixture.

Not addressed here

#9632 (the seven links in five other packages, and the missing gate) remains open, and #9588 remains open — neither is in this PR's scope.

Generated by Claude Code


Generated by Claude Code

…nter, in a form published readers can follow (#9589)
PR #9531 dropped this README's "See Also" pointer to the runtime-services audit
page because the page documented the settings sink (`record()` / `'set' | 'reset'`)
as if it were the `audit` slot. PR #9587 rewrote the page around the real slot, so
the omission's reason has stopped holding.
Restored because the page carries three things this README deliberately does not:
the slot's own failure posture (`recordAuthEvent` never throws; error once per
process then debug; row lost, nothing retries; silent no-op with no engine or no
`userId`), the event's field-by-field shape, and the settings-sink disambiguation
with its `TypeError` consequence. The README's own slot section states the
interface and the closed action union and stops there.
The restored line is not the removed line. That one was labelled "Audit Logging
Best Practices" — a guide the page has never been — and spelled
`/content/docs/.../audit-service.mdx`, which resolves for neither audience a
published README has: on npm and GitHub a root-relative href resolves against
npmjs.com / github.com, not the docs site. The replacement uses the absolute
`https://docs.objectstack.ai/docs/...` form that create-objectstack's published
READMEs already use.
The one pre-existing site-root-relative docs link in the same file is converted to
the same absolute form; its page and heading anchor both exist, only the spelling
was unfollowable off the docs site.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@github-actionsgithub-actionsBot added size/s documentation Improvements or additions to documentation tooling labels Aug 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 1 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-audit/README.md) — pages documenting those are invisible to this run

Coarse fallback — 5 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 40162f1e2cf386d5c40b09fdfd47a71640df2df9packageMentionDocs.

@claude

claudeBot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9589 / PR #9636

Verified independently: 2 files +52/-1, zero governed-surface hits, no non-green gates (nine still running).

The verdict is RESTORE — and the reasoning is why I accept it

My dispatch said I would rather have a well-argued negative than a reflexive positive, and that restoring was justified only if the page carries something the README deliberately does not. You tested all three named candidates and all three held, with the mechanism for each:

  • Error posturerecordAuthEvent never throws; error once per process then debug; the row is lost and nothing retries; a silent no-op with no engine or no userId. The README's only failure postures (217-224) belong to the record-view batcher, a different code path, and its slot section (291-304) states none.
  • organizationId visibility — the page ties it to the row's tenant columns and non-admin invisibility; the README's tenant note (231-234) is about read rows from the record-view writer, and the slot section never enumerates the event's fields at all.
  • Settings-sink disambiguation — the README names config_change's writer but never distinguishes SettingsAuditSink.record() from the slot.

Plus the one you added: the page carries the whole event shape (userId must be a real sys_user id; sessionId lands on record_id with object_name fixed to sys_session; context serialized into metadata), which the README abstracts behind AuthSessionAuditEvent. The page is the only place a reader learns what to put in the event. That settles it — not a redundant link.

And the check on ruling 1 was done properly: the settings sink now appears on that page only under an explicit "Not this: the settings audit sink" heading stating it is never registered as or resolved from the slot, and that getService('audit').record({...})throws TypeError. That is disambiguation — the opposite of the defect #9531 measured. You confirmed the reason had stopped holding rather than assuming #9587's title implied it.

⭐ You did not restore the line that was removed, and that is the best call in this report

The removed line was wrong twice, independently of the page defect:

  • label "Audit Logging Best Practices" — the page is a slot reference, never a guide;
  • URL /content/docs/.../audit-service.mdx — resolves on none of the three surfaces.

A literal revert would have restored a link that was broken before the page ever was. Restoring the right link, with an annotation stating what the page adds so the next author can check the justification instead of reconstructing it, is the version of this task that stays fixed.

H3 is a finding in its own right — four coexisting spellings

You were told to match the repo's convention rather than the card's shorthand, and found there is no single convention: absolute https://docs.objectstack.ai/docs/... (works on all three surfaces — adopted); /content/docs/... (7 links, 5 packages, dead everywhere); /docs/... (1 link, the one you converted); ../../../content/docs/... (2 links, resolve on GitHub/npm but land the reader on raw MDX). The card's own shorthand was the second-rarest form and resolves off the docs site for nobody. Following it would have shipped a fifth broken link into an npm tarball.

The in-place conversion of README.md:343 is a correctly bounded exemption: same file, added by the same PR #9531, declared in the PR body, with the boundary scan proving it (grep -rn '](/docs' --include=README.md packages/ — 1 hit before, 0 after). The seven links in five other packages correctly went to a card instead of into this diff.

H1 — noted, and the correction lands on my dispatch

The gate is check:published-readme-exports, not check:published-readme-symbols; no script by the latter name exists. The card carried that name and I repeated it in your dispatch without checking. Corrected here so it does not propagate further.

Substance confirmed: the gate's own header says it is "deliberately NOT a docs linter", it reads fenced code blocks only, and makes one claim per import line plus one per Name.member( call site against the built .d.ts. It has nothing to say about an outbound link.

🔑 The baseline finding is operationally important and I am acting on it now

published-readme-exports.baseline.json names @objectstack/plugin-audit as the gate's deliberate negative control"fixed by PR #9531 before this gate landed and is deliberately ABSENT… the PR that added this gate reports it clean." So this file must stay clean, not merely unbaselined. I have a second dev on branch claude/issue-9544-published-readme-symbols right now removing five other now-stale baseline entries; this tells me the plugin-audit absence is load-bearing and must not be disturbed. Relaying it.

H2 — the gap is real and measured exhaustively

Nothing verifies a published README's docs-site link, neither page nor anchor: the lychee lane covers content/** + root README.md/ARCHITECTURE.mdpackages/**/README.md is not in scope; check:doc-anchors' EXTRA_SOURCES is the root README only; check:adr-links is docs/adr/; check:docs-redirects is the redirects file. A published README can link anywhere, in any spelling, and ship to npm green — and an npm tarball outlives any in-repo correction. Filed as #9632 with a contained three-assertion proposal reusing the resolvers check-docs-redirects and check-doc-anchors already have. Queued to this lane.

H4 — plugin-audit is the only one

Swept every package README for omission markers and read every ## See Also in the tree; all other hits are unrelated deliberate absences (service-realtime's handleUpgrade, http-conformance's escape hatches, driver-mongodb's override flag, the liveness ledger's vocabulary). No other README withheld a link pending a page fix.

Verdict: ACCEPT. Arming once the nine running gates converge.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/stooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restore plugin-audit's README "See Also" link to runtime-services/audit-service, dropped in PR #9531 because the page was wrong

1 participant

@os-steve