feat(privacy): detect public-to-private visibility transitions in reconcile

[marcusrbrown]

Reconcile now compares each tracked entry's stored private flag against the live probe result. A false → true flip (including via access-lost, which fails closed as private) queues a reconcile:visibility-transition integrity-alert issue identifying the entry by node_id only — title and body both omit canonical owner/repo to keep the public issue stream safe.

The body includes a copy-paste gh api graphql command operators can run locally to resolve the node_id to its canonical owner/repo when investigating which wiki pages need manual review.

Durability

The privacy alert is one-shot: once metadata is committed private: true, the next reconcile run sees stored.private === probe.private === true and won't re-queue. Three failure modes that previously dropped the alert silently are now closed:

• paginate(issues.listForRepo) for dedup throws → fail open and create the issue anyway
• New reconcile:visibility-transition or reconcile:integrity-alert label not yet propagated to the live repo → pre-flight getLabel/createLabel; treat 422 from createLabel as the label-exists race
• Any label still can't be confirmed (non-404 getLabel or non-422 createLabel failure) → exclude that label from the payload; ship the issue with the surviving labels (or unlabeled with a loud warn) rather than dropping

Privacy posture

The implementation is stricter than the plan's permissive wording: even when the probe could resolve canonical owner/repo, the issue body omits it. Operators run the inline lookup command on demand. This matches the always-redacted-everywhere posture established by the survey workflow's visibility gate.

Defense-in-depth at the schema boundary: node_id must now match ^[\w\-+/=]+$, validated in both isRepoEntry and assertRepoEntry. Real base64ish GitHub IDs pass; shell metacharacters that would corrupt the operator's copy-paste are rejected at metadata-read time.

What stays open

Private → public transitions emit no alert; the next mutator write recanonicalizes the entry. Newcomers (no stored private field) emit no alert either — that's an initial categorization, not a flip. autoCloseStaleIssues is deliberately not extended to this label; operators close transition issues manually after acting.

Tests

649 passing + 3 todo (was 564 on main). New coverage includes:

• All 9 plan scenarios (false→true, false→false, true→true, true→false, undefined→true, false→access-lost, transient probe, rapid flip, node_id-only public surface)
• Privacy regression pin: rendered title and body never contain a canonical owner--repo slug (including dot-prefixed slugs like fro-bot--.github)
• Dedup branch coverage including a RED-against-old-substring-code regression test
• Three durability tests (paginate throws, label-404→createLabel→create, createLabel-422 vs createLabel-500 with payload inspection)
• Schema-level rejection test for node_id shell metacharacters

Follow-ups (deferred)

These are tracked separately rather than expanding this PR:

• Reconcile visibility-transition: same-run dedup race (multiple alerts per node_id) #3332 same-run dedup race (apply the PR fix(reconcile): suppress self-heal rollup duplicates from same-run race #3321 same-run Set pattern to visibility transitions)
• Reconcile: visibilityTransitions counter missing from operator-readable Step Summary #3333 surface visibilityTransitions counter in the reconcile Step Summary
• Reconcile visibility-transition: duplicate-skip is silent (no counter, no log) #3334 emit a counter/log when dedup suppresses creation
• Reconcile visibility-transition: legacy entries without node_id have no alertable subject #3335 legacy entries without node_id have no alertable subject when access is revoked
• Reconcile visibility-transition: 'rapid flip' test is a duplicate single-probe test #3336 rename the 'rapid flip' test to match what it actually tests
• Reconcile visibility-transition: counter accuracy in ambiguous create-error paths #3337 document or tighten counter accuracy in ambiguous issues.create error paths

[fro-bot]

Verdict: PASS

Clean privacy work. The defense-in-depth here — node_id-only public surface, fail-open dedup, label pre-flight with 422-as-success, schema-level node_id sanitization — closes the three silent-drop failure modes called out in the PR description without leaking canonical identity into the public issue stream. The exhaustiveness guard in runIssueQueue is the right shape for keeping future IssueQueueEntry additions honest.

The false → true flip with undefined excluded as "initial categorization" is the correct sticky semantics. access-lost failing closed as private is consistent with the survey workflow's posture.

Blocking issues

None.

Non-blocking concerns

• scripts/reconcile-repos.ts:2528 — the inline gh api graphql command in the rendered body embeds ${issue.node_id} inside single quotes. The schema regex ^[\w\-+/=]+$ correctly rejects single quotes, backticks, $, ;, and whitespace at metadata-read time, so this is safe today. The safety is implicit in two distant files coupled by a regex though — a one-line comment at the render site pointing at the schema invariant (or vice versa) would make the contract auditable without chasing references. Not blocking; the schema test pins the invariant.
• scripts/reconcile-repos.ts:~2475 — ensureLabelsExist issues two sequential getLabel/createLabel round-trips per visibility-transition issue even on the steady-state happy path (labels already exist via .github/settings.yml). For the expected cardinality (at most a handful of transitions per run), the latency is negligible. Worth noting only because the cost scales linearly with transition count; if you ever see a thundering herd, cache the confirmed set across calls within one handleReconcile invocation.
• scripts/reconcile-repos.test.ts:~525 — the privacy regression pin uses /[\w.-]+--[\w.-]+/ to assert no canonical slug. This catches the canonical owner--repo form but would not catch a owner/repo form if a future renderer change introduced one. The companion test on line ~535 (.not.toContain('fro-bot/tracked-repo')) covers the slash form for the specific fixture, but a generic \w+\/[\w.-]+ regression pin would be sturdier against unknown future leak shapes. Optional hardening.
• Follow-up #3335 (legacy entries without node_id have no alertable subject when access is revoked) is the right deferral — maybeQueueVisibilityTransition correctly skips when node_id === undefined, so the silent drop is bounded to legacy entries that pre-date node_id capture. Acceptable as long as #3335 stays open.

Missing tests

None. Coverage is thorough: all 9 plan scenarios, privacy regression pin (including dot-prefixed slugs), exact-title dedup with the RED-against-substring proof, paginate-throws fail-open, label 404 → createLabel call ordering, 422-as-success vs 500-as-exclude with payload inspection, and schema rejection of shell metacharacters with both new-style and legacy base64 acceptance. The dedup branch coverage in particular is well-constructed — the R_kgDOZAAAA vs R_kgDOZAAAAA test is the kind of regression pin that actually prevents recurrence.

Risk assessment: LOW

Reasoning:

• Engine-side change is additive and gated on storedPrivate === false — no behavior change for entries already marked private or newcomers.
• Issue-creation path is idempotent at the GitHub layer (exact-title dedup) and resilient to transient failures (fail-open dedup, label pre-flight with 422 race handling, graceful payload degradation to unlabeled).
• Schema tightening on node_id is strict-but-correct: real base64ish GitHub IDs pass, shell metacharacters fail at metadata-read time. Worst case for a malformed-but-previously-accepted entry: assertReposFile throws on next run, which is the intended fail-loud behavior.
• No workflow/CI changes. No new dependencies. No any, no @ts-expect-error, no console (uses params.logger). Action SHA pinning N/A.
• Privacy posture stricter than the plan specified (always omits canonical owner/repo even when resolvable), which is the right call for a public issue stream.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	26151057003
Cache	hit
Session	ses_1bb7b78e5ffeKPqzxmNm2UXq5U

[fro-bot]

Verdict: PASS

Re-review after my prior approval was dismissed. Commit 6e4f9d3 cleanly addresses both NBCs from the first pass:

• Schema-render contract is now auditable. The cross-reference comment at scripts/reconcile-repos.ts:2528 points at the schema invariant (/^[\w\-+/=]+$/) that makes the inline gh api graphql interpolation shell-safe, and scripts/schemas.ts:259 points the other direction. Two distant files no longer coupled by an implicit regex — the contract is documented at both ends.
• Generic slash-form privacy pin landed. The added expect(payload.body).not.toMatch(/(?!owner\/repo)\b\w[\w-]*\/[\w.-]+/) in scripts/reconcile-repos.test.ts catches unknown future leak shapes while the (?!owner\/repo) lookahead correctly excludes the prose placeholder in the operator instructions. Verified empirically: fro-bot/test matches, owner/repo does not, nameWithOwner does not, and the actual rendered body produces no match.

Everything else from the prior review still holds: defense-in-depth posture (node_id-only public surface, fail-open dedup, label pre-flight with 422-as-success, schema-level node_id sanitization), exhaustiveness guard in runIssueQueue, sticky false → true flip semantics with undefined excluded as initial categorization, no workflow/CI churn, no new deps, no banned constructs.

Blocking issues

None.

Non-blocking concerns

• scripts/reconcile-repos.ts:~2475 — ensureLabelsExist still issues two sequential getLabel/createLabel round-trips per visibility-transition issue even on the steady-state happy path where labels already exist via .github/settings.yml. For the expected cardinality of a handful of transitions per run this is negligible; cache the confirmed set across calls within one handleReconcile invocation only if you ever observe a thundering herd.
• Follow-up #3335 (legacy entries without node_id have no alertable subject when access is revoked) remains the right deferral — maybeQueueVisibilityTransition correctly skips when node_id === undefined, so the silent drop is bounded to pre-node_id entries. Acceptable as long as #3335 stays open.

Missing tests

None. Coverage is unchanged from the prior pass plus the new generic slash-form regression pin. Plan scenarios, dedup branch coverage (including the RED-against-substring proof), durability triplet (paginate-throws, label 404→createLabel→create call ordering, 422-vs-500 payload inspection), and schema rejection with new-style and legacy base64 acceptance are all present.

Risk assessment: LOW

Reasoning:

• Engine-side change is additive and gated on storedPrivate === false — no behavior change for entries already private or for newcomers.
• Issue-creation path is idempotent at the GitHub layer (exact-title dedup) and resilient to transient failures (fail-open dedup, label pre-flight with 422 race handling, graceful payload degradation to unlabeled).
• Schema tightening on node_id is strict-but-correct: real base64ish GitHub IDs pass, shell metacharacters fail at metadata-read time. Worst case for a malformed-but-previously-accepted entry: assertReposFile throws on next run, which is the intended fail-loud behavior.
• No workflow/CI changes. No new dependencies. No any, no @ts-expect-error, no console (uses params.logger). Action SHA pinning N/A.
• Privacy posture stricter than the plan specified (always omits canonical owner/repo even when resolvable), which is the right call for a public issue stream.

---

Run Summary

Field	Value
Event	pull_request
Repository	fro-bot/.github
Run ID	26151452280
Cache	hit
Session	ses_1bb7b78e5ffeKPqzxmNm2UXq5U