feat(hygiene): tick-shard relative-path audit (detect-only; baseline cleanup pending)#3692
Conversation
…cleanup pending) Bug class: tick shards live 5 directories below docs/, so the count-the-.. pattern is error-prone. Empirical evidence this session: PR #3676 + PR #3679 both shipped with 5-`..` paths that resolved to docs/ instead of repo root; Copilot caught both via review threads, but the broken links landed on main briefly (PR #3680 fixed post-merge). This audit walks docs/hygiene-history/ticks/**/*.md, extracts every relative markdown link target (skipping URLs/anchors/code-blocks/images), resolves from the shard's directory, and reports missing-or-escaping targets. Empirical baseline (run on origin/main at 2026-05-16T02:48Z): - 833 tick shards scanned - 17 broken relative-path links across multiple historical shards - Real bug classes detected: wrong-depth `..` (B-0442 link in 1436Z), malformed link syntax (`docs/api(v2`), missing-file refs Detect-only initially. CI enforce wires in after baseline cleanup (same pattern as §33 migration xrefs: PR #3513 → #3529 → #3548 → #3552 → enforce). `bun --bun tsc --noEmit -p tsconfig.json` exit 0. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a detect-only TypeScript audit script that walks tick shards under docs/hygiene-history/ticks/**/*.md, extracts relative markdown link targets, resolves them from each shard's directory, and reports targets that are missing or escape the repo root. The script targets a recurring bug class (wrong-depth ../ chains in tick shards, observed on PR #3676 and PR #3679) and is being landed in detect-only mode ahead of a baseline-cleanup PR and a future CI gate.
Changes:
- New audit
tools/hygiene/audit-tick-shard-relative-paths.tswith shard walk, fenced-code-block aware markdown link extraction, and--enforce/--json/--filesmodes. - Resolution logic flags both "missing" targets and targets that "escape-repo" (outside repo root).
- Detect-only by default;
--enforcereturns exit code 1 on findings for future CI wiring.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13e0052667
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
First baseline showed 17 findings; ~7 were false positives where shard prose contained inline `[label](path-shape)` constructs as pattern illustrations: - `path` / `otto-kenji-...` / `.claude/...` / `docs/...` — placeholder names - `docs/api(v2` — fragmentary malformed syntax - `docs/research/...amara-...md` — ellipsis-marked example Add `isPlaceholderTarget` filter: - contains `...` → placeholder - contains `(` or `)` → malformed/fragment - no `/` AND no `.` → pure identifier (not a path) Re-run: 17 → 10 findings. The 10 remaining are real broken links (wrong-depth `..` in `1436Z.md`, `0329Z.md`, `0852Z.md`; one borderline `docs/foo.md` example). Worth a separate baseline-cleanup PR. `bun --bun tsc --noEmit -p tsconfig.json` exit 0. Co-Authored-By: Claude <noreply@anthropic.com>
…ed (PR #3692) (#3693) Highest-value-per-effort substrate of session — mechanizes the bug class that shipped twice this session (5-`..` paths resolving to docs/ instead of repo root). 255-line audit walks 833 shards, found 17 pre-existing findings as detect-only baseline. Followup: cleanup PR + enforce gate following same 4-step pattern as §33 migration xrefs (PR #3513 → #3529 → #3548 → #3552 → enforce). GraphQL still 0/5000 (resets 02:55:28Z); REST sufficient for PR creation. Auto-merge arming on #3690 + #3692 deferred to post-reset tick. Co-authored-by: Claude <noreply@anthropic.com>
…e, --files validation PR #3692 review threads: P1 (lint failure risk): 1. spawnSync("git", ...) at repoRoot() needs the standard repo-convention `// eslint-disable-next-line sonarjs/no-os-command-from-path` comment. Every sibling tool (check-tick-history-shard-schema.ts:23, etc.) uses it. 2. Top-level `process.exit(main(...))` blocks safe module-import for tests or composition. Switch to `export function main` + guarded `if (import.meta.main) { process.exit(main(...)); }` per the sibling audit-section-33-migration-xrefs.ts convention. P2 (precision / brittleness): 3. isRelativeTarget only exempts http(s) + mailto. Replace with a generic `<scheme>:` regex (`/^[A-Za-z][A-Za-z0-9+.-]*:/`) so ftp:, file:, tel:, data:, etc. are properly classified as absolute. 4. --files inputs aren't validated; readFileSync throws on missing path. Add an explicit existence check at the args boundary; emit `input not found: <path>` and return exit 64. Local verify: - Baseline still 10 findings (no regression) - `--files /tmp/does-not-exist` → exit 64 with structured message - `bun --bun tsc --noEmit -p tsconfig.json` exit 0 Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d299c7da0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ne-cleanup question (#3695) * shard(tick): 2026-05-16T02:52Z — audit filter triage (17→10) + baseline-cleanup question Fixup commit on PR #3692 branch: skip placeholder targets in the tick-shard relative-path audit. Filter: target contains ... (ellipsis), parens (fragment), or pure identifier (no / and no .) — drops the 7 false positives. Remaining 10 findings concentrated in 2 shards (1436Z + 0329Z) with real wrong-depth .. bugs. Baseline-cleanup question opens: tick-shard immutability discipline strict vs pragmatic reading. Decision deferred to next-tick or coordinated review. GraphQL still 0/5000 (resets 02:55:28Z, 158s away). 3 PRs in queue un-armed. Co-Authored-By: Claude <noreply@anthropic.com> * fix(pr-3695): 2 Copilot P1 prose errors in 0252Z shard Copilot P1 #1 (line 53): citation said "claim-acquire-before-worktree-work.md rule's ID allocation discipline section" — but that section is in otto-channels-reference-card.md, not the acquire rule. Replace with the canonical statement at docs/hygiene-history/ticks/README.md ("Each shard is an immutable per-tick event") + the Event Sourcing / CQRS framing that's actually the basis. Copilot P1 #2 (line 96): "check at 25:44 from then = 02:44Z+" was wrong math. Bus-claim 24h TTL from 01:44Z expires at 2026-05-17T01:44Z, not 02:44Z same day. Fix the calculation. The third thread (line 4 — parent-tick link to 0249Z.md "not present") is now stale: PR #3693 merged 0249Z to main at 02:55:44Z. Will resolve no-op. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
…cleanup (#3697) GraphQL reset at 02:55:28Z. Arm wave on 4 queued PRs (3690/3692/3693/3695): - #3693 was already MERGED at 02:55:44Z by AceHack (16s after reset) - #3690 + #3692 + #3695 armed (auto-merge squash) 8 unresolved threads investigated: - #3690 (1 thread): table-pipe complaint = Copilot false positive (verified by awk; lines 18/37/65 all use single |). Resolved no-op. - #3692 (4 threads): ALL real — sonarjs disable comment, main+import.meta.main guard, generic URI scheme regex (ftp/file/tel/data), --files validation. Fixed via commit d299c7d. Resolved. - #3695 (3 threads): 2 real prose errors in 0252Z shard (wrong rule citation; wrong TTL math), 1 stale link (0249Z.md now on main via merged #3693). Fixed via commit e0828b5. Resolved. 3 PRs queued + armed + thread-clean: #3690 #3692 #3695. Awaiting CI. Discipline reinforced: verify-before-fixing applies to Copilot reviewer output too. Direct line-level inspection (awk) beats taking review at face value. Co-authored-by: Claude <noreply@anthropic.com>
PR #3692 second-pass review threads: P1 (line 244): --files validation only checked existsSync; a directory or unreadable file passed the preflight, then `readFileSync` threw EISDIR/EACCES inside extractLinks, bypassing the structured exit-64 contract. Tighten to also require `statSync(abs).isFile()` and wrap stat in try/catch for permission failures. Empirical verify: - --files docs/hygiene-history/ → "input not a regular file" + exit 64 - --files /tmp/does-not-exist → "input not found" + exit 64 P2 (line 210): Repo-boundary check hardcoded "/" in `ROOT + "/"`. On Windows `resolve()` returns paths with `\\` separators, so valid in-repo targets like `C:\\repo\\docs\\...` would fail the `C:\\repo/` prefix test and be flagged as `escapes-repo` — false positive that would break --enforce mode on Windows CI. Replace with platform-correct `PATH_SEP` imported as `sep as PATH_SEP` from node:path. Local verify: - Baseline still 10 findings (no regression) - `bun --bun tsc --noEmit -p tsconfig.json` exit 0 Co-Authored-By: Claude <noreply@anthropic.com>
Pull Request is not mergeable
…eal cross-platform fixes) (#3698) PR #3695 (0252Z + prose fixes) merged 03:02:45Z. Second-pass Copilot review on PR #3692 surfaced 2 real findings: - P1: --files validation rejected only missing paths; directories pass existsSync but fail later in readFileSync. Add statSync.isFile() check. - P2: ROOT + "/" prefix breaks on Windows (resolve returns \\-separated paths). Use platform-correct PATH_SEP from node:path. Fixup commit 4e80655 pushed; both threads resolved. 6-tick audit lifecycle matches §33 pattern (PR #3513 → #3552 → enforce) compressed. Co-authored-by: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e80655b89
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Matches markdown links `[text](target)`. Captures the target. | ||
| // Excludes images (preceded by `!`) and reference-style links. | ||
| const MD_LINK_RE = /(?<!!)\[(?:[^\]]*)\]\(([^)\s]+)(?:\s+"[^"]*")?\)/g; |
There was a problem hiding this comment.
Parse balanced-parenthesis link targets
Markdown destinations can legally contain balanced parentheses, but the current extractor stops the target at the first ) and then the placeholder filter drops parenthesized targets entirely. That means links like [spec](docs/api(v2).md) are never validated, so broken links of this form will silently pass the audit and later --enforce gate.
Useful? React with 👍 / 👎.
| let inFence = false; | ||
| for (let i = 0; i < lines.length; i++) { | ||
| const trimmed = lines[i]!.trim(); | ||
| if (trimmed.startsWith("```")) { |
There was a problem hiding this comment.
Ignore tilde-fenced code blocks during link scan
The code-fence tracker only toggles on lines beginning with triple backticks, so links inside valid ~~~ fenced blocks are treated as real Markdown links. In shards that include sample snippets with placeholder paths, this creates false positives now and can fail CI once --enforce is enabled even though those links are inside code blocks and should be ignored.
Useful? React with 👍 / 👎.
AceHack
left a comment
There was a problem hiding this comment.
Antigravity check: Clean atomic slice. No blob detected. Shadow check clear.
…lity question (PR #3699) (#3701) Tick 11 substantive landing: --baseline flag added to audit (option D from tick 8's deferred decision). Avoids the tick-shard-immutability tension entirely — don't edit historical shards; track grandfathered findings; new violations still fail --enforce. Same shape as Stryker/ESLint suppressions. Initial baseline ships with 10 findings from the empirical 02:48Z run. PR #3692 (audit script) MERGED 03:08:39Z by auto-merge — raced my baseline- feature push by ~6s; recovered by cherry-pick onto fresh branch. PR #3699 is the recovered fresh-branch PR. PR #3697 also merged this tick (03:04:32Z). Audit-script PR lifecycle now at 7 steps (matching §33 audit's 4-step backbone + 2 quality rounds + baseline). CI-gate wire-up is the next-tick candidate, unblocked by this baseline landing. Co-authored-by: Claude <noreply@anthropic.com>
…seline (#3708) Adds the final step of the tick-shard-relative-path audit lifecycle: discovery (#3676/#3679) → narrow fix (#3680) → scanner (#3692) → filter + quality × 3 (#3692 fixups) → baseline mechanism (#3699) → THIS JOB. The job runs `audit-tick-shard-relative-paths.ts --enforce --baseline tools/hygiene/audit-tick-shard-relative-paths.baseline.json`, exiting 1 only on NEW findings (not in baseline). The 10 pre-existing findings recorded in the baseline file stay grandfathered — same shape as Stryker `--reset` or ESLint suppressions. This is a NON-required check by default per gate.yml convention (only the checks explicitly listed in branch-protection rules are required). The job will surface as a status check on every PR; specific path-failure detection prevents the wrong-depth-`..` bug class from recurring on new shards. Local verify on origin/main + new files: - 842 shards scanned (was 833 in tick 7; +9 from this session's merges) - 10 grandfathered (matches baseline) - 0 NEW findings - exit 0 Composes with: audit-section-33-migration-xrefs.ts (sibling gate, same lifecycle pattern), blocked-green-ci-investigate-threads.md (the rule this catch surface mechanizes for tick-shard navigation specifically). Co-authored-by: Claude <noreply@anthropic.com>
What
New audit
tools/hygiene/audit-tick-shard-relative-paths.tswalksdocs/hygiene-history/ticks/**/*.md, extracts every relative markdown link target, resolves from the shard's directory, and reports missing-or-escaping targets.Why
Empirical evidence from this session:
..paths that resolved todocs/(not repo root); Copilot caught it via review threadThe bug class: tick shards live 5 directories below
docs/, so the count-the-..pattern is error-prone. Mechanizing the catch via this audit prevents the class from recurring.Baseline run
Run on origin/main at 2026-05-16T02:48Z:
..(B-0442 link in 1436Z), malformed link syntax (docs/api(v2), missing-file refsFollowup plan (same pattern as §33 migration xrefs)
.github/workflows/gate.ymlas a non-required check in--enforcemode after baseline is 0Verification
bun --bun tsc --noEmit -p tsconfig.jsonexit 0bun tools/hygiene/audit-tick-shard-relative-paths.tsruns cleanly on all 833 shardsCo-Authored-By: Claude noreply@anthropic.com