fix(disk-hygiene): scan transcript head for guard-launch failures

[kyle-sexton]

Fixes #1514

Test plan

• python3 -m pytest plugins/disk-hygiene/skills/clean/scripts/test_guard_launch_monitor.py -k head_region_failure

Related

N/A

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[claude]

Claude finished @kyle-sexton's task in 2m 18s —— View job

---

Security Review

• Fetch PR diff
• Read changed files
• Analyze for security issues
• Post findings

This PR changes guard_launch_monitor.py's _read_tail() to also scan the transcript's head region (not just the last 2 MB) so guard-launch failures aren't missed when a later turn pushes them out of the tail window. Reviewed plugins/disk-hygiene/skills/clean/scripts/guard_launch_monitor.py, test_guard_launch_monitor.py, plugin.json, and CHANGELOG.md (the only files this PR touches). No secrets/injection/auth issues found, but one logic flaw undermines the module's own stated security-reliability invariant.

---

IMPORTANT — Head-region scan reintroduces the unbounded-cost problem this detector was explicitly designed to avoid
guard_launch_monitor.py#L179-L194

The module docstring (lines 47–58) is explicit that this detector must stay O(cap), not O(session length), citing a prior real incident (D-12: a hook that cost 12–19s p50 per call across ~1,464 runs) as the reason it deliberately reads only a bounded tail. The new head-scan branch breaks that invariant:

head_limit = size - _MAX_TAIL_BYTES
head_raw = handle.read(head_limit)   # reads the ENTIRE file minus 2MB

Critically, per main()#L273-L290 and _run()#L252-L270, the once-per-session marker is written only when a guard failure is actually found. In the normal/expected case — no guard failures — the marker never gets written, so this full-head read repeats on every single Stop event for the rest of the session, re-reading and re-decoding an ever-growing prefix of the transcript each turn (effectively O(n²) over a long session). This is exactly the D-12 cost shape the module's docstring says it exists to avoid, just moved from PreToolUse to Stop.

There's a secondary consequence: main() wraps everything in except BaseException and fails silently closed (line 288-289). A very large transcript (multi-GB sessions are explicitly anticipated by the docstring — "Transcript files grow for the life of a session") reading handle.read(head_limit) into memory can raise MemoryError, which this except-all swallows, so the detector could silently stop warning on exactly the large, long-running sessions where a missed guard failure matters most — quietly reintroducing the original silent-failure class (#1416) this whole module exists to close, now via resource exhaustion instead of a crashed hook launch.

Confidence: High — verified directly in the code; the marker-only-on-found gating is unconditional and not test-covered for the repeated-scan/no-failure path.

Suggested fix: cap the head scan too (e.g., only scan back some bounded multiple of _MAX_TAIL_BYTES, or persist a byte offset already-scanned so each turn only re-scans newly appended head bytes), and/or write the once-per-session marker regardless of outcome once a full scan has completed successfully.

Fix this →

---

SUGGESTION — Boundary record between head and tail is silently dropped
guard_launch_monitor.py#L165-L194

The JSONL record that straddles the head/tail split point is discarded from both sides: the trailing partial line is trimmed off head_raw (line 191), and _read_tail_from_handle's mid-record detection then also discards the remainder of that same record from the start of the tail window (handle.readline() at line 174). If that dropped record happens to be the only hook_non_blocking_error guard-failure attachment in the transcript, it is silently lost — the exact silent-suppression failure mode this detector exists to prevent (per the module's own framing in lines 20-23, 73-77). This is a narrow, low-probability edge case (one JSONL line out of the whole transcript) and mirrors a pre-existing trade-off already present in the original tail-only code, so I'm flagging it as a suggestion rather than a hard finding.

Confidence: Medium — the mechanics are verified in code; likelihood of a real guard failure landing exactly on this byte boundary is low.

---

No other security issues (injection, auth, secrets handling, or GitHub Actions risk) found in this PR's changes — plugin.json and CHANGELOG.md are non-functional metadata edits, and the file continues to be parsed as untrusted-but-inert JSON text with no eval/shell/subprocess usage.

Branch: cursor/guard-launch-head-scan-63e1

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2b8e5c050e

ℹ️ 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".

[claude]

Claude finished @kyle-sexton's task in 1m 32s —— View job

---

Security Review

• Fetch PR diff
• Read changed files
• Analyze for security issues
• Post findings

This PR changes guard_launch_monitor.py's _read_tail() to also scan the transcript's head region (not just the last 2 MB) so guard-launch failures aren't missed when a later turn pushes them out of the tail window. Reviewed all four changed files: guard_launch_monitor.py, test_guard_launch_monitor.py, plugin.json, CHANGELOG.md. No injection, auth, secrets-handling, or GitHub Actions issues — this PR touches no workflow files, and plugin.json/CHANGELOG.md are non-functional metadata edits. One logic flaw undermines the module's own stated security-reliability invariant.

---

IMPORTANT — Head-region scan reintroduces the unbounded per-Stop cost this detector exists to avoid, and can silently disable the detector on large transcripts

guard_launch_monitor.py#L179-L194

The module docstring (lines 47-58) states this detector must stay O(cap), not O(session length), citing D-12 (a hook that cost 12-19s p50 across ~1,464 runs) as the reason it deliberately bounds its read. The new branch breaks that invariant:

head_limit = size - _MAX_TAIL_BYTES
head_raw = handle.read(head_limit)   # reads the entire transcript minus 2MB

The once-per-session marker is written only on the path where a failure is found (main()#L281-L286, gated by _run()'s if not failures: return None at L268-L269). In the common case — no guard failures — the marker never gets written, so this full-head read repeats on every Stop event for the rest of the session, re-reading and re-decoding an ever-growing prefix of the transcript each turn (O(n²) over a long session). This is the same D-12 cost shape the docstring says the module exists to avoid, just moved from PreToolUse to Stop.

There's a compounding failure mode: main() wraps everything in except BaseException and fails closed silently (L288-L289). On a very large transcript (multi-GB sessions are explicitly anticipated by the docstring's "Transcript files grow for the life of a session"), handle.read(head_limit) loading gigabytes into memory can raise MemoryError, which this except-all swallows — the detector would silently stop warning about guard failures on exactly the largest, longest-running sessions, where a missed guard failure matters most. That's a reintroduction of the original silent-failure class (#1416) this module exists to close, now via resource exhaustion rather than a crashed hook launch.

Confidence: High — verified directly in the code; the marker-only-on-found gating is unconditional, and the repeated full-head-read path has no bound or offset persistence.

Suggested fix: cap the head scan too (e.g., only scan back some bounded multiple of _MAX_TAIL_BYTES, or persist a scanned-offset so each turn only re-scans newly appended head bytes), and/or write the once-per-session marker once a full scan has completed successfully, regardless of outcome.

Fix this →

---

SUGGESTION — Boundary record between head and tail is silently dropped

guard_launch_monitor.py#L165-L194

The JSONL record straddling the head/tail split point is discarded from both sides: the trailing partial line is trimmed off head_raw (L190-L191), and _read_tail_from_handle's mid-record detection then also discards the remainder of that same record from the start of the tail window (handle.readline() at L174). If that dropped record is the only hook_non_blocking_error guard-failure attachment in the transcript, it is silently lost — the exact silent-suppression failure mode this detector exists to prevent (per the module's own framing, lines 20-23). This is a narrow, low-probability edge case (one JSONL line out of the whole transcript) and mirrors a pre-existing trade-off already present in the original tail-only code, so flagging as a suggestion rather than a hard finding.

Confidence: Medium — mechanics verified in code; likelihood of a real guard failure landing exactly on this byte boundary is low.

---

No other security issues found. This PR does not touch any GitHub Actions workflow files, so no pull_request_target/workflow_run/permissions/pin concerns apply here.

Branch: cursor/guard-launch-head-scan-63e1

[claude]

Claude finished @kyle-sexton's task in 2m 20s —— View job

---

Security Review

• Fetch PR diff
• Read changed files
• Analyze for security issues
• Post findings

This PR changes _read_tail() in guard_launch_monitor.py to also scan the transcript's head region (not just the last 2 MB) so guard-launch failures aren't missed when a later turn pushes them past the tail window. I independently re-read all four changed files (guard_launch_monitor.py, test_guard_launch_monitor.py, plugin.json, CHANGELOG.md) against HEAD f2cebb7. No injection, auth, secrets-handling, or GitHub Actions issues — this PR touches no workflow files, and plugin.json/CHANGELOG.md are non-functional metadata edits. One logic flaw undermines the module's own stated reliability invariant; confirming the finding already raised by Codex and prior review passes on this PR.

---

IMPORTANT — Head-region scan reintroduces the unbounded per-Stop cost this detector exists to avoid

guard_launch_monitor.py#L179-L194

The module docstring (L47-L58) states the read must stay O(cap), not O(session length), citing D-12 (a hook that cost 12-19s p50 across ~1,464 runs) as the reason. The new branch breaks that invariant:

head_limit = size - _MAX_TAIL_BYTES
head_raw = handle.read(head_limit)   # reads the entire transcript minus 2 MB

The once-per-session marker is written only when a failure is actually found (main() L281-L286, gated by _run()'s if not failures: return None at L268-L269). In the common case — no guard failures in the session — the marker never gets written, so this full-head read repeats on every Stop event for the rest of the session, re-reading and re-decoding an ever-growing prefix of the transcript each turn (O(n²) over a long session). This is the same D-12 cost shape the docstring says the module exists to avoid, just moved from PreToolUse to Stop.

Compounding this: main() wraps everything in except BaseException and fails closed silently (L288-L289). On a very large transcript (multi-GB sessions are explicitly anticipated by the docstring's "Transcript files grow for the life of a session"), handle.read(head_limit) loading gigabytes into memory can raise MemoryError, silently swallowed here — the detector would stop warning on exactly the largest, longest-running sessions, where a missed guard failure matters most. That's a reintroduction of the original silent-failure class (#1416) this module exists to close, now via resource exhaustion rather than a crashed hook launch.

Confidence: High — verified directly against current HEAD; the marker-only-on-found gating is unconditional and the repeated full-head-read path has no bound or persisted offset. Also independently flagged by Codex (P1 comment) and prior automated passes on this PR.

Suggested fix: cap the head scan too (e.g., only scan back a bounded multiple of _MAX_TAIL_BYTES, or persist a scanned-offset so each turn only re-scans newly appended head bytes), and/or write the once-per-session marker once a full scan has completed, regardless of outcome.

Fix this →

---

SUGGESTION — JSONL record straddling the head/tail split point is silently dropped

guard_launch_monitor.py#L179-L194

Tracing the byte ranges precisely: head_raw = handle.read(head_limit) consumes file bytes [0, head_limit) and advances the handle to head_limit. If that read doesn't end on a \n, the trailing partial line is trimmed from head_raw in memory (L190-L191) — but the file handle is not seeked back, so those trimmed prefix bytes are never re-read. _read_tail_from_handle is then called with size=size - head_limit, which equals _MAX_TAIL_BYTES exactly, so its own if size > _MAX_TAIL_BYTES mid-record guard (L166) never fires, and it just reads raw from the current position (head_limit) to EOF — i.e. the suffix of the same split record, with no leading context. Net effect: the record's prefix bytes are discarded from head_text, and its suffix becomes an unparsable JSON fragment at the start of tail_text, so json.loads fails on it and it's silently skipped by the except (json.JSONDecodeError, ValueError): continue in _iter_guard_failures. If that one record happens to be the session's only hook_non_blocking_error guard-failure attachment, it's silently lost — the exact suppression class this detector exists to prevent (per the module's own framing, L20-L23). This is a narrow, low-probability edge case (one JSONL line landing exactly on the split byte) and mirrors a pre-existing trade-off already present in the original tail-only code, so flagging as a suggestion rather than a hard finding.

Confidence: Medium — mechanics verified by tracing exact byte offsets in the current code; likelihood of a real guard failure landing exactly on this boundary is low.

---

No other security issues found. This PR does not touch any GitHub Actions workflow files, so no pull_request_target/workflow_run/permissions/pin concerns apply.

Branch: cursor/guard-launch-head-scan-63e1

[claude]

Claude finished @kyle-sexton's task in 5m 23s —— View job

---

Code Review

• Gather context (diff, changed files, REVIEW.md)
• Review guard_launch_monitor.py changes
• Review test_guard_launch_monitor.py changes
• Check CHANGELOG.md / plugin.json version bump
• Post findings as inline comments + summary

This lane covers correctness, design, error handling, tests, and documentation; security/performance findings (already reported on this PR by the security lane) are intentionally left to it.

One Important finding, posted inline on guard_launch_monitor.py lines 165-193:

_read_tail always calls _read_tail_from_handle(handle, size=size - head_limit), and since head_limit = size - _MAX_TAIL_BYTES, that size argument is algebraically always exactly _MAX_TAIL_BYTES — so the function's own if size > _MAX_TAIL_BYTES: guard is always False on this call path, and the mid-record discard logic it gates can never run in production. When the head/tail split lands inside a JSONL record (the typical case, since head_limit has no relation to line boundaries), that record is silently dropped from both sides instead of retained on either — the opposite of a narrow edge case, since it's the common shape of the boundary, not a rare one. If the straddling record is the session's only guard-failure attachment, the warning is lost, which cuts against this PR's stated goal. This path also has no test coverage: the existing test_tail_window_starting_on_a_record_boundary_keeps_that_record test only exercises the old single-window logic with the boundary deliberately placed on a clean newline. Full details and a suggested fix (re-seek the handle after trimming head_raw so the tail read resumes exactly where the head text left off) are in the inline comment.

Otherwise: the plugin.json version bump (0.17.2 → 0.17.3) and CHANGELOG.md entry are consistent with the fix; the new test (test_head_region_failure_survives_later_oversized_append) is well-constructed for the case it targets (failure record isolated near the very start of the file, far from the split boundary), it just doesn't happen to cover the boundary case above.

Verification note: I did not have working shell access to run pytest in this session, so the finding above is based on static/algebraic tracing of the code (confirmed independent of runtime data), not test execution. I have not independently re-run the author's stated test plan.

[github-actions]

Claude has reviewed this PR 4 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

[claude]

Claude finished @cursor[bot]'s task in 2m 31s —— View job

---

Security Review

• Gather context (diff, changed files)
• Invoke /review:security-review skill (skill file read directly; the Skill tool errored on this repo's local review plugin, so I followed plugins/review/skills/security-review/SKILL.md manually)
• Post findings

Reviewed all four changed files at HEAD db7018b449a209d328938dd870a819df3f66f3c1: guard_launch_monitor.py, test_guard_launch_monitor.py, plugin.json, CHANGELOG.md. No injection, auth, secrets-handling, or GitHub Actions issues — this PR touches no workflow files, and plugin.json/CHANGELOG.md are non-functional metadata edits.

I independently re-verified the core logic in _read_tail/_read_tail_from_handle at this HEAD and it is byte-for-byte unchanged from the commit reviewed earlier on this PR (only the version/changelog bump differ) — both previously-reported logic flaws still apply here and are unfixed. Posted as inline comments, summarized below:

IMPORTANT — _read_tail L185-194: the head-region scan (handle.read(head_limit), effectively the whole transcript minus 2 MB) repeats on every Stop event for the rest of a clean session, because the once-per-session marker is only written when a failure is actually found. This reintroduces the O(session length) per-Stop cost the module's own docstring (L47-58) says it exists to avoid (citing D-12: 12-19s p50 across ~1,464 runs), and a MemoryError on a very large transcript is silently swallowed by main()'s blanket except BaseException.

IMPORTANT — _read_tail/_read_tail_from_handle L165-193: _read_tail always calls _read_tail_from_handle(handle, size=size - head_limit), and since head_limit = size - _MAX_TAIL_BYTES, that size argument is algebraically always exactly _MAX_TAIL_BYTES — so the if size > _MAX_TAIL_BYTES mid-record guard at L166 can never fire on this call path, making its seek-back/discard logic dead code. A JSONL record straddling the head/tail split (the routine case, since head_limit has no relation to line boundaries) is silently dropped from both sides. If that record is the session's only hook_non_blocking_error attachment, the warning is lost — undercutting this PR's own purpose and the CHANGELOG's claim that failures "outside the 2 MB tail window are no longer lost." No test in this PR covers a record straddling head_limit.

Both findings were already surfaced by Codex and by earlier review passes on this PR; confirming they remain unresolved at the current head.
· Branch: cursor/guard-launch-head-scan-63e1

[github-actions]

Last security-reviewed head: 6406da15a9e519b2a3654ec8cae55fb9825066fa. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

[claude]

Claude finished @cursor[bot]'s task in 4m 2s —— View job

---

Code Review

• Gather context (diff, changed files, REVIEW.md/CLAUDE.md)
• Review guard_launch_monitor.py changes
• Review test_guard_launch_monitor.py changes
• Check CHANGELOG.md / plugin.json version bump
• Post findings as inline comments + summary

This lane covers correctness, design, error handling, tests, and documentation. Per REVIEW.md, security/performance findings are scoped to the dedicated security lane, which has already reviewed this PR in depth on this same underlying change — I did not re-report the unbounded head-scan/DoS finding it raised.

One Important finding, posted inline on guard_launch_monitor.py lines 165-193:

_read_tail always calls _read_tail_from_handle(handle, size=size - head_limit), and since head_limit = size - _MAX_TAIL_BYTES, that size argument is algebraically always exactly _MAX_TAIL_BYTES — so the helper's own if size > _MAX_TAIL_BYTES: guard is always False on this call path, and the mid-record discard logic it gates can never run. Because head_limit has no relation to JSONL line boundaries, it typically lands mid-record — the boundary record's prefix is trimmed from head_raw in memory without the file handle being seeked back, so the tail read resumes at the wrong offset and the record's suffix becomes an unparsable fragment that's silently skipped by _iter_guard_failures. If that record is the session's only hook_non_blocking_error attachment, the guard-failure warning is lost — cutting directly against this PR's stated goal and the new CHANGELOG entry's claim. No test in this PR constructs a transcript where head_limit lands inside a record (the existing boundary test deliberately lands on a clean newline; the new test isolates its failure record far from the split), so this regression isn't caught. Full trace and a suggested fix (re-seek after trimming head_raw, plus a boundary test that straddles head_limit on a non-newline byte) are in the inline comment.

This is the same bug already identified and inline-commented by a prior automated pass on this PR (on an earlier HEAD); it remains present, unaddressed, on the current HEAD (db7018b).

Otherwise: the plugin.json version bump (0.17.3 → 0.17.4) and CHANGELOG.md entry are consistent in form with the fix; test_head_region_failure_survives_later_oversized_append is a well-constructed test for the case it targets (failure record isolated near the start of the file, far from any split boundary).

Verification note: I did not have working shell access to execute pytest in this session (only git, Read, Glob, Grep were available), so the finding above is based on static/algebraic tracing of the code against the current HEAD, not test execution. I have not independently re-run the author's stated test plan.

Branch: cursor/guard-launch-head-scan-63e1

[claude]

Claude finished @kyle-sexton's task in 53s —— View job

---

Security Review

• Gather context (diff, changed files)
• Invoke /review:security-review skill
• Analyze for security issues
• Post findings

Skip gate: this head already has a security review that still applies.

HEAD 77bfee5dc3fceba6c678e0edadd766e4e9864719 is a merge of main into this branch. I diffed it against db7018b449a209d328938dd870a819df3f66f3c1 (the commit the previous security-review pass on this PR already reviewed) restricted to this PR's four changed files:

git diff db7018b..77bfee5 -- plugins/disk-hygiene/.claude-plugin/plugin.json \
  plugins/disk-hygiene/CHANGELOG.md \
  plugins/disk-hygiene/skills/clean/scripts/guard_launch_monitor.py \
  plugins/disk-hygiene/skills/clean/scripts/test_guard_launch_monitor.py

— empty output. The merge pulled in unrelated changes to other plugins (autonomy, bash-format, education, guardrails, powershell-format, rate-limit-guard, session-flow, songwriting, work-items, etc.) but none of them touch this PR's diff; guard_launch_monitor.py and the other three files in this PR are byte-for-byte unchanged since the last review.

Per the skill's skip gate (plugins/review/skills/security-review/SKILL.md), no new review is needed here. For reference, the previously-reported findings on this code (still unresolved as of db7018b) were:

• IMPORTANT — the head-region scan (_read_tail, guard_launch_monitor.py L179-194) reads effectively the whole transcript minus 2 MB on every Stop event for the rest of a clean session, reintroducing the O(session length) per-hook cost the module's docstring says it exists to avoid, with a MemoryError on very large transcripts silently swallowed by main()'s blanket except BaseException.
• IMPORTANT — _read_tail always calls _read_tail_from_handle(handle, size=size - head_limit), which is algebraically always exactly _MAX_TAIL_BYTES, so the if size > _MAX_TAIL_BYTES mid-record guard in _read_tail_from_handle (L165-193) can never fire on this call path, making its seek-back/discard logic dead code. A JSONL record straddling the head/tail split (the routine case) is silently dropped from both sides — if that record is the session's only hook_non_blocking_error attachment, the warning is lost.

Neither finding has code changed since it was reported, so I have not re-posted duplicate inline comments.

Branch: cursor/guard-launch-head-scan-63e1

[claude]

Claude finished @cursor[bot]'s task in 1m 30s —— View job

---

Security Review

• Gather context (diff, changed files, HEAD SHA)
• Invoke /review:security-review skill instructions
• Analyze for security issues
• Post findings

---

Skip gate: this head already has a security review that still applies.

HEAD 3fd79e49136ef6387f5d7e6dead7c4055e2326f7 diffed against db7018b449a209d328938dd870a819df3f66f3c1 (the last commit with a full independent security review on this PR) and against 77bfee5dc3fceba6c678e0edadd766e4e9864719 (the last commit explicitly marked as "security-reviewed" for this PR), restricted to this PR's four functional files:

git diff 77bfee5..3fd79e4 -- plugins/disk-hygiene/.claude-plugin/plugin.json \
  plugins/disk-hygiene/CHANGELOG.md \
  plugins/disk-hygiene/skills/clean/scripts/guard_launch_monitor.py \
  plugins/disk-hygiene/skills/clean/scripts/test_guard_launch_monitor.py

— empty output. The only changes since then are unrelated docs/changelog updates merged in from main (docs/conventions/loop-lane/CHANGELOG.md, docs/conventions/loop-lane/README.md, plugins/source-control/.claude-plugin/plugin.json version bump, plugins/source-control/CHANGELOG.md, plugins/source-control/skills/pull-request/reference/monitor.md) — none of which touch this PR's actual code, and none of which are GitHub Actions workflow files.

Per the security-review skill's skip gate, no new review is needed. For reference, the previously-reported findings on guard_launch_monitor.py (still unresolved as of this HEAD) were:

• IMPORTANT — the head-region scan (_read_tail, L179-194) reads effectively the whole transcript minus 2 MB on every Stop event for the rest of a clean session, reintroducing the O(session length) per-hook cost the module's own docstring says it exists to avoid (citing D-12: 12-19s p50 across ~1,464 runs), with a MemoryError on very large transcripts silently swallowed by main()'s blanket except BaseException.
• IMPORTANT — _read_tail always calls _read_tail_from_handle(handle, size=size - head_limit), which is algebraically always exactly _MAX_TAIL_BYTES, so the if size > _MAX_TAIL_BYTES mid-record guard in _read_tail_from_handle (L165-193) can never fire on this call path, making its seek-back/discard logic dead code. A JSONL record straddling the head/tail split (the routine case, since head_limit has no relation to line boundaries) is silently dropped from both sides — if that record is the session's only hook_non_blocking_error attachment, the warning is lost, cutting against this PR's own CHANGELOG claim.

Neither finding has had code changed since it was reported, so no duplicate inline comments were posted.

Branch: cursor/guard-launch-head-scan-63e1

[claude]

Claude finished @cursor[bot]'s task in 3m 35s —— View job

---

Code Review (/review:code-review)

• Gather context (diff via gh pr diff, changed files)
• Check skip gate (has this head already been reviewed by this lane?)
• Review new/unreviewed files in the diff
• Post findings

Skip gate (partial): the disk-hygiene code is unchanged since the last code-review pass on this PR.

git diff db7018b..HEAD restricted to this PR's four disk-hygiene files (plugin.json, CHANGELOG.md, guard_launch_monitor.py, test_guard_launch_monitor.py) is empty — byte-for-byte identical to the commit this lane already reviewed twice (17:59 and 19:19 UTC passes above). Per the skip gate, I'm not re-reviewing or re-posting duplicate findings there. For reference, the previously-posted unresolved finding still applies verbatim at the same lines on current HEAD:

• guard_launch_monitor.py#L165-L193 — _read_tail (L179-194) always calls _read_tail_from_handle(handle, size=size - head_limit) at L193, and since head_limit = size - _MAX_TAIL_BYTES (L188), that size argument is algebraically always exactly _MAX_TAIL_BYTES. The if size > _MAX_TAIL_BYTES: guard at L166 is therefore always False on this call path, so the seek-back/discard logic at L167-174 is dead code from this call site. Because head_limit has no relation to JSONL line boundaries, a record straddling the split is routinely dropped from both sides — if that's the session's only hook_non_blocking_error attachment, the guard-failure warning is lost, cutting against this PR's own purpose. Confirmed I verified this again by reading the current file directly (unchanged from the prior pass).

New review surface (not previously covered by this lane): loop-lane and source-control docs

The diff also carries docs/conventions/loop-lane/{CHANGELOG,README}.md and plugins/source-control/{.claude-plugin/plugin.json,CHANGELOG.md,skills/pull-request/reference/monitor.md} — unrelated to this PR's stated disk-hygiene scope (these match separately-merged PRs #2512/#2513 pulled in via merge-forward). I reviewed them for correctness/consistency since they're part of the diff:

• loop-lane/CHANGELOG.md's 9.0.0 entry claims "§2 gains a Cross-lane PR hold subsection" — verified: README.md §2 (Escalation contract) does contain the new ### Cross-lane PR hold subsection at the expected location.
• source-control/CHANGELOG.md's 0.53.14 entry references monitor.md's stale-branch recovery rule "in §3.2" — verified: the merge-forward guidance is correctly placed under ## 3.2 CI failure resolution.
• The new statusCheckRollup jq selectors (failed vs. still-running) are syntactically valid and match the documented pitfall.

No correctness or maintainability issues found in these files — no findings to post.

Result: no new findings from this lane. The one outstanding Important finding on guard_launch_monitor.py (dead mid-record-discard branch causing silent data loss at the head/tail split) remains unresolved from prior passes; see the security lane's parallel findings on the unbounded head-scan cost for the performance/DoS angle.
· Branch: cursor/guard-launch-head-scan-63e1

[claude]

Claude finished @cursor[bot]'s task in 53s —— View job

---

Security Review

• Gather context (diff, changed files, HEAD SHA)
• Invoke /review:security-review skill instructions
• Analyze for security issues
• Post findings

---

Skip gate: this head already has a security review that still applies.

HEAD 6406da15a9e519b2a3654ec8cae55fb9825066fa diffed against 3fd79e49136ef6387f5d7e6dead7c4055e2326f7 (the last commit explicitly marked as security-reviewed for this PR), restricted to this PR's four functional files:

git diff 3fd79e4..6406da1 -- plugins/disk-hygiene/.claude-plugin/plugin.json \
  plugins/disk-hygiene/CHANGELOG.md \
  plugins/disk-hygiene/skills/clean/scripts/guard_launch_monitor.py \
  plugins/disk-hygiene/skills/clean/scripts/test_guard_launch_monitor.py

— empty output. The only changes since then are unrelated content merged in from main (formatting-plugin hook-utils.sh updates, songwriting research/skill docs, work-items tooling, session-flow skills, various CHANGELOG.md/plugin.json version bumps across other plugins) — none of which touch this PR's actual code, and none of which are GitHub Actions workflow files.

Per the security-review skill's skip gate (item 3: "this head already has a successful security review that still applies"), no new review is needed. For reference, the previously-reported findings on guard_launch_monitor.py (still unresolved as of this HEAD) were:

• IMPORTANT — the head-region scan (_read_tail, L179-194) reads effectively the whole transcript minus 2 MB on every Stop event for the rest of a clean session, reintroducing the O(session length) per-hook cost the module's own docstring says it exists to avoid (citing D-12: 12-19s p50 across ~1,464 runs), with a MemoryError on very large transcripts silently swallowed by main()'s blanket except BaseException.
• IMPORTANT — _read_tail always calls _read_tail_from_handle(handle, size=size - head_limit), which is algebraically always exactly _MAX_TAIL_BYTES, so the if size > _MAX_TAIL_BYTES mid-record guard in _read_tail_from_handle (L165-193) can never fire on this call path, making its seek-back/discard logic dead code. A JSONL record straddling the head/tail split (the routine case, since head_limit has no relation to line boundaries) is silently dropped from both sides — if that record is the session's only hook_non_blocking_error attachment, the warning is lost, cutting against this PR's own CHANGELOG claim.

Neither finding has had code changed since it was reported, so no duplicate inline comments were posted.

Branch: cursor/guard-launch-head-scan-63e1

[claude]

Claude finished @cursor[bot]'s task in 1m 36s —— View job

---

Code Review (/review:code-review)

• Gather context (diff, changed files, skip gate)
• Check skip gate against last-reviewed head
• Post findings

Skip gate: this head already has a code-review pass from this lane that still applies.

HEAD 6406da15a9e519b2a3654ec8cae55fb9825066fa diffed against 3fd79e49136ef6387f5d7e6dead7c4055e2326f7 (the head this lane last confirmed as unchanged since its full review at db7018b449a209d328938dd870a819df3f66f3c1), restricted to this PR's four files:

git diff 3fd79e4..6406da1 -- plugins/disk-hygiene/.claude-plugin/plugin.json \
  plugins/disk-hygiene/CHANGELOG.md \
  plugins/disk-hygiene/skills/clean/scripts/guard_launch_monitor.py \
  plugins/disk-hygiene/skills/clean/scripts/test_guard_launch_monitor.py

— empty output. The 83 files touched between those two commits are all unrelated content pulled in via merge-forward from main (songwriting/pat-pattison research docs, source-control, typos-format, and work-items plugin bumps/hooks) — none of them intersect this PR's disk-hygiene scope. guard_launch_monitor.py and the other three files remain byte-for-byte identical to the commit this lane already reviewed twice.

Per the skip gate, I'm not re-running the review or re-posting duplicate inline comments. For reference, the one Important finding from this lane's prior pass remains unresolved at the same lines on current HEAD:

• guard_launch_monitor.py#L165-L193 — _read_tail (L179-194) always calls _read_tail_from_handle(handle, size=size - head_limit) at L193, and since head_limit = size - _MAX_TAIL_BYTES (L188), that size argument is algebraically always exactly _MAX_TAIL_BYTES. The if size > _MAX_TAIL_BYTES: guard at L166 is therefore always False on this call path, so the seek-back/readline()-discard logic at L167-174 is dead code from this call site. Because head_limit has no relation to JSONL line boundaries, it routinely lands mid-record — the boundary record's prefix is trimmed from head_raw in memory without the file handle being seeked back, so the tail read resumes at the wrong offset and the record's suffix becomes an unparsable fragment silently skipped by _iter_guard_failures's except (json.JSONDecodeError, ValueError): continue. If that record is the session's only hook_non_blocking_error attachment, the warning is lost — cutting directly against this PR's stated purpose and the CHANGELOG's claim that failures "outside the 2 MB tail window are no longer lost." No test in this PR constructs a transcript where head_limit lands inside a record. Suggested fix: after trimming head_raw, seek the handle back by the number of trimmed bytes so the tail read resumes exactly where head_text left off, then add a boundary test that straddles head_limit on a non-newline byte.

(Separately, the security lane has already flagged the unbounded-cost issue with the head-region scan reading effectively the whole transcript on every clean Stop event — that's out of scope for this lane per REVIEW.md's security/performance split.)

No new findings from this lane — the merge-forward content outside this PR's stated scope was not reviewed, consistent with prior passes scoping strictly to the disk-hygiene files this PR touches.

Branch: cursor/guard-launch-head-scan-63e1