fix(scheduler): recover truncated history without losing valid records

[SebTardif]

Recover interrupted scheduler history writes without losing valid records. Current main rejects a truncated final JSON value, blocking crawlctl run, status, and logs. The original recovery proposal treated every missing newline as truncation, which also hid and later deleted valid JSON records at EOF.

The corrected recovery distinguishes a genuinely incomplete terminal JSON value from a valid record or complete corrupt JSON. Valid EOF records retain their original bytes and receive a newline separator before the next record is appended. Complete corrupt records still fail closed. Reads remain streaming, and appends inspect only the trailing record instead of loading the whole history file. The file uses read/write access and an explicit append seek under the existing scheduler lock: Windows append-only handles cannot truncate. A partial write is rolled back to the pre-append length; write, rollback, and close failures are returned to the caller.

Validation:

• Added regressions reproduced valid-EOF data loss and hidden terminal corruption against the original PR before the fix.
• Tests cover every truncated byte boundary of an encoded record, valid EOF/CRLF across read blocks, malformed and wrong-type JSON, actual partial bytes written to temp files, short writes, failed rollback, and close errors.
• make check and GOWORK=off go test -count=1 ./... passed, including vet, deadcode, govulncheck, full tests/race tests, and 25 release guard tests.
• Built CLI comparison used current main, the original PR, and the fixed candidate with temporary configs/history/logs. It reproduced both bugs and verified status/logs/run, unchanged bytes during reads, preserved prior rows and inserted separators during append, and rejection of complete corruption.
• A real RLIMIT_FSIZE kernel write failure made the fixed CLI report “file too large,” restore the prior valid EOF record byte-for-byte, release its lock, and successfully append on the next unrestricted run.
• Initial Windows CI exposed the append-handle truncation permission defect. The follow-up uses read/write access and explicit seek; it retains the same regression coverage.
• Full-candidate independent Codex P0–P2 review against main cf646f20a72664166107fa06d4ee2a41b3e439af passed without actionable findings.

Dependency PR #106 was integrated by normal merge commit cad6c55. Both changelog entries and the contributor's original commit remain in the branch. The combined candidate passed make check, actual CLI/kernel-write-failure proof, and a separate Go 1.27.0 storage/encryption consumer using SQLite 1.58.0 and its exact libc 1.75.6 pairing. Thanks @SebTardif (Sebastien Tardif) for the recovery fix and initial proof. The maintainer follow-up preserves that credit in its commit and the Unreleased changelog. Related prior proposal: #99.

The pre-existing Scanner record-size limit remains unchanged. A built-CLI comparison confirms both pre-fix main and this candidate reject valid and incomplete 70 KiB final records with the same scanner error; increasing supported record sizes is outside this fix.

No live archives, release workflows, or installed schedules were modified.

Final combined head cad6c5533020e391c647196e78dd9bda0e1ae6cc, base c43a1e1b14f35cf4fec49e78c8d4d76fa789074e: Linux/Windows CI, CodeQL, and secret scan all passed. Combined full P0–P2 review passed with no actionable findings; uncached full Go tests also passed.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs changes before merge. Reviewed September 4, 2026, 5:21 AM ET / 09:21 UTC.

ClawSweeper review

What this changes

The PR repairs crawlctl scheduler-history recovery after interrupted JSONL writes while preserving valid final records without a newline and surfacing write cleanup errors.

Merge readiness

⛔ Needs changes before merge - 2 items remain

Keep open: the prior valid-EOF data-loss blocker is fixed, but the recovery path still cannot process an interrupted final record over Scanner’s 64 KiB default limit, leaving scheduler commands blocked for that valid input shape.

Priority: P2
Reviewed head: 18a5527ec136ca12e8263b4c432470f4f193e0f7

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The patch has strong focused coverage and credible real CLI proof, but one bounded recovery-path defect remains.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The PR body documents real crawlctl execution with temporary configuration and local history, including an injected kernel file-size failure with observed rollback, lock release, and later recovery; this directly exercises the changed append path, though not the remaining oversized-tail case.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The PR body documents real crawlctl execution with temporary configuration and local history, including an injected kernel file-size failure with observed rollback, lock release, and later recovery; this directly exercises the changed append path, though not the remaining oversized-tail case.
Evidence reviewed	6 items	Recovery reader remains size-limited: The introduced reader still constructs bufio.Scanner with its default token limit. An oversized unterminated final JSON record causes Scanner to stop before the split callback can classify it as recoverable. History rows can contain unbounded command data: Job.Command has no configured size bound and is copied into every RunRecord before serialization, so a legitimate record can exceed Scanner’s default maximum token size. Current tests stop below the Scanner boundary: The valid EOF regression uses a 5,000-character field and the interrupted-prefix regression uses a small marshaled record; neither proves recovery once Scanner rejects the terminal line before JSON decoding.
Findings	1 actionable finding	[P2] Handle history tails beyond Scanner’s 64 KiB limit
Security	None	None.

How this fits together

crawlctl’s scheduler records completed refresh jobs in a local JSONL history file. The run, status, and logs commands consume that history, while a scheduled run appends the next record under a single-process lock.

flowchart LR
  Jobs[Configured refresh jobs] --> Run[Scheduler run]
  Run --> History[Local JSONL history]
  History --> Reader[History reader]
  Reader --> Tail[Validate final record]
  Tail --> Repair[Repair incomplete tail]
  Repair --> Commands[Run status and logs]

Loading

Before merge

• Handle history tails beyond Scanner’s 64 KiB limit (P2) - ReadHistory still uses the default bufio.Scanner limit, so an unterminated final RunRecord over 64 KiB returns a scanner error before the new split callback can identify it as incomplete. Job.Command is unbounded and is serialized into history, so this leaves run, status, and logs blocked for a case this PR is intended to recover. Raise the reader limit or use a streaming decoder, and cover a >64 KiB partial final line.
• Complete next step (P2) - Make terminal-history recovery handle records above Scanner’s default 64 KiB limit and add a focused regression before merge.

Findings

• [P2] Handle history tails beyond Scanner’s 64 KiB limit — scheduler/run.go:378-386

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch surface	4 files affected: production +99/-4, tests +266	Most of the change is focused regression coverage around one local scheduler-history path.

Root-cause cluster

Relationship: canonical
Canonical: #103
Summary: This PR is the active successor to an earlier closed, unmerged scheduler-history proposal.

Members:

• superseded: fix(scheduler): write run history as one complete JSONL line #99 - The earlier proposal only made new writes complete; this PR adds recovery of pre-existing interrupted tails and supersedes that incomplete approach.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Make terminal-history recovery independent of Scanner’s default token ceiling, then retain the current valid-EOF, corruption, rollback, and Windows-handle guarantees.

Do we have a high-confidence way to reproduce the issue?

Yes, from source: configure a job whose serialized RunRecord exceeds 64 KiB, leave an interrupted final JSON value, then invoke scheduler run, status, or logs; Scanner errors before the new recovery branch can handle it.

Is this the best way to solve the issue?

No. The patch correctly addresses valid EOF records and ordinary interrupted tails, but it must also handle a terminal record beyond Scanner’s default limit for the advertised recovery behavior to hold.

Full review comments:

• [P2] Handle history tails beyond Scanner’s 64 KiB limit — scheduler/run.go:378-386
ReadHistory still uses the default bufio.Scanner limit, so an unterminated final RunRecord over 64 KiB returns a scanner error before the new split callback can identify it as incomplete. Job.Command is unbounded and is serialized into history, so this leaves run, status, and logs blocked for a case this PR is intended to recover. Raise the reader limit or use a streaming decoder, and cover a >64 KiB partial final line.
  Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against cf646f20a726.

Labels

Label changes:

• remove merge-risk: 🚨 compatibility: Current PR review selected no merge-risk labels.

Label justifications:

• P2: The remaining failure affects local scheduler recovery for an uncommon but valid large-history-record input, without evidence of an emergency regression.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The PR body documents real crawlctl execution with temporary configuration and local history, including an injected kernel file-size failure with observed rollback, lock release, and later recovery; this directly exercises the changed append path, though not the remaining oversized-tail case.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body documents real crawlctl execution with temporary configuration and local history, including an injected kernel file-size failure with observed rollback, lock release, and later recovery; this directly exercises the changed append path, though not the remaining oversized-tail case.

Evidence

Acceptance criteria:

• [P1] GOWORK=off go vet ./...
• [P1] GOWORK=off go test -count=1 ./...

What I checked:

• Recovery reader remains size-limited: The introduced reader still constructs bufio.Scanner with its default token limit. An oversized unterminated final JSON record causes Scanner to stop before the split callback can classify it as recoverable. (scheduler/run.go:378, 18a5527ec136)
• History rows can contain unbounded command data: Job.Command has no configured size bound and is copied into every RunRecord before serialization, so a legitimate record can exceed Scanner’s default maximum token size. (scheduler/config.go:30, 18a5527ec136)
• Current tests stop below the Scanner boundary: The valid EOF regression uses a 5,000-character field and the interrupted-prefix regression uses a small marshaled record; neither proves recovery once Scanner rejects the terminal line before JSON decoding. (scheduler/scheduler_test.go:352, 18a5527ec136)
• Scheduler feature history: History identifies the scheduler’s original implementation in d41253c and the current follow-up commits that corrected the previous valid-EOF behavior and Windows handle mode. (scheduler/run.go:279, d41253c66810)
• Real CLI proof supplied: The PR body describes temporary-config crawlctl runs and a kernel RLIMIT_FSIZE failure, with byte-preserving rollback, lock release, and a subsequent successful append; it exercises the changed local-history behavior through the CLI. (18a5527ec136)
• Related earlier proposal: fix(scheduler): write run history as one complete JSONL line #99 was a closed, unmerged predecessor that did not recover already-truncated history; this PR is its focused successor.

Likely related people:

• unknown: The claimed source-line change could not be verified from bounded local history. (role: source history unknown; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Add a >64 KiB interrupted-terminal-record regression that proves run, status, and logs recover after the reader change.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (7 earlier review cycles)

• reviewed 2026-09-02T20:28:37.946Z sha ec1be60 :: needs changes before merge. :: [P1] Preserve valid final JSON without a newline
• reviewed 2026-09-02T21:59:43.467Z sha ec1be60 :: blocked before merge. :: [P1] Preserve valid final JSON without a newline
• reviewed 2026-09-03T08:00:34.479Z sha ec1be60 :: blocked before merge. :: [P1] Retain valid terminal JSON records without a newline
• reviewed 2026-09-03T12:01:07.754Z sha ec1be60 :: blocked before merge. :: [P1] Validate terminal JSON before truncating it
• reviewed 2026-09-03T15:53:31.249Z sha ec1be60 :: blocked before merge. :: [P1] Preserve valid terminal JSON records without a newline
• reviewed 2026-09-04T02:53:30.000Z sha ec1be60 :: blocked before merge. :: [P1] Preserve valid terminal JSON records without a newline
• reviewed 2026-09-04T08:58:57.815Z sha ec1be60 :: blocked before merge. :: [P1] Preserve valid terminal JSON records without a newline