docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept - #14761

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields
Sep 3, 2026
Merged

docs(sharing): say why getPolicy's disabled-branch redactFields read is kept#14761
os-sales merged 2 commits into
mainfrom
claude/issue-14581-getpolicy-disabled-redactfields

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes#14581

Comment-only. getPolicy()'s enabled !== true branch keeps its raw.redactFields read; the comment beside it now describes why, instead of describing a case that can no longer arise.

⚠️ This takes the card's route 2, and my dispatch asked for route 1

Flagging it up front, because it is a disposition disagreement and not a judgement call I should bury. The dispatch that sent me here selected route 1 (collapse the branch to redactFields: []). The triage comment on the card, posted five hours earlier by a different seat, had already ruled route 2 explicitly — "leave the read, rewrite the comment", "⛔ Comment only. No behaviour change, no test change." The dispatch does not mention that ruling.

I took route 2. It is what the prior ruling of record says, and my dispatch left the door open for it ("you may better it, with reasons"; "if you conclude route 2 is right anyway, argue it from what you measured"). The measurement below is the argument, and it points the same way. A maintainer who wants route 1 after all should say so — nothing here is hard to revisit.

Premise checks, run before editing, at origin/main7a17f3bf1

1 · Is the read actually dead? Yes, exactly as the card claims.

  • policy.redactFields has one reader in the file — the union at :772 in resolveToken.
  • The [#14033] gate if (!policy.enabled) sits at :697 and return nulls, 75 lines above that reader.
  • createLink (:442) reads enabled (:449) and at :545 writes input.redactFields — the caller's list, never policy.redactFields.
  • getPolicy has exactly two callers repo-wide, both in this file (:442, :650). Every other hit of the name is a comment, changelog, or docs reference.
  • share-link-routes.ts reads resolved.redactFields, which is resolveToken's return — only ever non-null on the enabled path.

2 · Does removing it red anything? No — and that is the finding. See below.

3 · Is getPolicy exported? No. It is a module-private function getPolicy(schema: any) at :89. The package entry exports ShareLinkService and its options type, not this helper. Clause ② stays no.

The measurement that decided the route

I applied route 1's exact collapse as an ablation, proved it reached disk (blob 726dace8 to b017e772; injected marker count 1, removed-text count 0, via a globalThis write rather than a comment), and ran the suite:

route-1 collapse applied: Test Files 30 passed (30) Tests 726 passed (726)

Byte-identical to baseline. Nothing reds. Restore proven afterwards by blob equality against HEAD plus an empty git diff HEAD.

A green ablation is ambiguous on its own — it can also mean the mutated line never executed. So the branch was proven live with a second, deliberately-fatal mutation in the same spot (throw new Error('ABLATION_PROBE_14581'), also proven on disk):

disabled branch throws: Test Files 2 failed | 28 passed (30) Tests 18 failed | 708 passed (726)

The branch runs 18 times across the suite, and not one of those 18 assertions cares which value it returns. That is what makes the green above a measurement rather than an artefact. Restored again, blob equality plus empty git diff HEAD.

That result cuts both ways, and the second way is the load-bearing one:

  • it confirms the read is unreachable today, so route 1 would have been safe now;
  • it also proves no pin distinguishes the two shapes. The four #14171 pins that used to read the set with the switch off are all reversed to toBeNull(), so they pass under either version. If the :697 gate ever regresses, route 1 restores #13856's fail-open widening with zero test coverage to catch it; route 2 still fails closed.

Deleting a free, fail-closed read whose absence no test would notice, in a file where #14637 currently measures a stated security property being defeated one layer up, is the trade that looks clean in the diff and bad in the incident. The read stays; the comment now says exactly that, so it is documented defence in depth rather than dead code wearing a confident wrong comment.

Why the comment is dense: line-count parity is deliberate

First pass wrote this over 32 lines. That rotted check-system-context-census, which anchors this file's context.isSystem sites by absolute line number — measured, all three legs:

treecensus
base, unmodifiedOK — ... 145 anchors resolve (exit 0)
base + 32-line comment10 problem(s) over 145 anchors (exit 1)
--fixrewrites 5 anchors, all on content/docs/permissions/system-context.mdx:138

One line of churn — but that page is one of the two hottest generated files in the repo and is contended by two other open PRs on this branchline (#14528, #14726). A comment-only change carrying no behaviour should not need a census regenerated to land, so the replacement says the same five things in exactly the 11 lines it replaced. File is 1006 lines before and after; no anchor moves; the census stays green without being touched.

Clause ② — no, derived from the diff

$ git diff -U0 origin/main...HEAD | grep -E '^\+\s*export '
(no matches)

That grep cannot see a changed signature on an already-exported symbol, so that half is checked separately and mechanically: stripping // comment lines from the base file and from this one yields byte-identical content (sha 12e7b2f64695a3a3 both sides). No declaration, signature, or statement moved — a comment-only diff cannot alter an export surface.

Tests

before: Test Files 30 passed (30) Tests 726 passed (726)
after: Test Files 30 passed (30) Tests 726 passed (726)

pnpm --filter @objectstack/plugin-sharing test plus typecheck (check:test-typecheck: OK), both on d2b749a92, tree clean. The named control pin — control — the enabled:true path serves exactly declared ∪ per-link, as before — passes untouched; it reads the enabled branch at :122, which this diff does not go near.

Gates

24 commands (23 derived by dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the always-owed check:nul-bytes), every exit code captured before any pipe. 21 exit 0. Zero real findings. Three non-zero, all NOT MEASURED on their own words:

  • check-test-completeness (exit 3) — "PREREQUISITE NOT MET — this gate grades a saved turbo run test log, and no log was named ... ⛔ It is not a red"
  • check:dual-build-cjs-loads (exit 3) — "PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/ ... ⛔ This is NOT a pass: nothing was measured"
  • check:i18n (exit 1, and its verdict is why the code is not the classifier) — "Nothing was checked: no bundle was compared and no config was parsed"

All three want a full workspace build that a comment-only diff cannot influence; CI builds and runs them properly.

Changeset

None, deliberately. Comment-only, no behaviour change, and the compiled output is unchanged — this releases nothing from any package, so skip-changeset is applied rather than an empty changeset, which check-empty-changeset.mjs rejects outright.

Left alone on purpose

🤖 Generated with Claude Code

https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8

Generated by Claude Code


Generated by Claude Code

…is kept
`getPolicy()`'s `enabled !== true` branch reads `raw.redactFields`. Its
comment justified that read by a case #14033 removed: it spoke of "tokens
that still serve" on a switched-off block, and after #14033's redemption
gate no such token exists. The comment was describing an impossible case,
which is how a read outlives the reason anyone can still read for it.
Measured before rewriting it, on `origin/main` 7a17f3b:
- `policy.redactFields` has exactly one reader in the file — the union in
`resolveToken` — and the `[#14033]` gate returns `null` 75 lines above it.
- `createLink` never reads it on any branch; the row it writes carries the
caller's `redactFields`.
- `getPolicy` is module-private with exactly two callers, both in-file.
So the read is unreachable, as the card says. Collapsing the branch back to
`redactFields: []` was also measured: the whole `@objectstack/plugin-sharing`
suite stays green, 726/726, unchanged. That is the reason the read is KEPT
rather than removed — no pin distinguishes the two shapes, so a future
regression of the gate would restore #13856's fail-open widening uncaught,
and the read is the only thing that fails closed behind it.
Comment only. Non-comment content of the file is byte-identical.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
…he base
The first pass wrote the same explanation over 32 lines where the comment it
replaced used 11. That is not free: `check-system-context-census` anchors the
`context.isSystem` read sites in this file by ABSOLUTE LINE NUMBER, so +21
lines rotted 10 of them and turned the gate red. Measured both ways on
`origin/main` 7a17f3b:
- base file, unmodified -> `check-system-context-census: OK ... 145 anchors
resolve` (exit 0)
- base + the 32-line comment -> `10 problem(s) over 145 anchors`, five
`[site-without-a-row]` and five `[anchor-is-not-a-read-site]` (exit 1)
- `--fix` repairs it by rewriting 5 anchors, all on ONE line
(`content/docs/permissions/system-context.mdx:138`)
One line of churn is small, but that page is one of the two hottest generated
files in the repo and is contended by two other open PRs on this branchline.
A comment-only change that carries no behaviour should not need a census
regenerated to land, so the comment now says the same five things in exactly
the 11 lines it replaced. The file is 1006 lines before and after, no anchor
moves, and the census stays green without being touched.
Non-comment content remains byte-identical to the base.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AUF1NoViznQK32gqpK8wS8
@claudeclaudeBot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Sep 3, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 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.

Coarse fallback — 8 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 7a17f3bf1e48e1e88f8e833a794e37bd40230f17packageMentionDocs.

Which tree this was computed on

This run read content/docs from a1d6677a8e466a8b54b16313f99bc19207db731a — the merge of head d2b749a92d72ec0e0ff72b33edab7feb60166a03 into base 7a17f3bf1e48e1e88f8e833a794e37bd40230f17, 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 a1d6677a8e466a8b54b16313f99bc19207db731a && git checkout a1d6677a8e466a8b54b16313f99bc19207db731a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 d2b749a92d72ec0e0ff72b33edab7feb60166a03 && git checkout -B drift-repro 7a17f3bf1e48e1e88f8e833a794e37bd40230f17 && git merge --no-ff d2b749a92d72ec0e0ff72b33edab7feb60166a03
node scripts/docs-audit/affected-docs.mjs --json 7a17f3bf1e48e1e88f8e833a794e37bd40230f17

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

@os-sales
os-sales marked this pull request as ready for review September 3, 2026 00:09
@os-sales
os-sales added this pull request to the merge queueSep 3, 2026
Merged via the queue into main with commit d686f45Sep 3, 2026
42 checks passed
@os-sales
os-sales deleted the claude/issue-14581-getpolicy-disabled-redactfields branch September 3, 2026 02:02
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

finding: getPolicy()'s disabled-branch redactFields read has no reader once publicSharing.enabled is held at redemption (#14033)

2 participants

@os-sales@claude