fix(threading): guard the pool cancellation flag behind a lock

[frostney]

Summary

• Fixes both findings from fix(review): CodeRabbit round-2 findings across the stack #1101's review, one of them a data race the round-2 layer introduced: the pool's shared cancel flag was a raw PBoolean read and written across threads with no synchronization (disassembly: bare ldrb/strb, no barriers), so a worker could observe a stale False after the first failure and keep draining the queue.
• It becomes a small lock-owning TGocciaCancellationFlag on the unit's existing critical-section idiom — the Boolean is private, reachable only through three locked methods, so no future access site can bypass it. All seven sites are covered (worker read/write, pool init, RunAll reset, Cancel, the Cancelled property getter, worker handoff).
• Bonus defect closed: the old @FCancelled pointed into the pool object, so an abandoned watchdog-stalled worker kept dereferencing it after Pool.Free — a use-after-free every caller could hit. The flag now follows the work queue's established leak rule.
• Evidence, since a passing suite proves nothing about races: 400 stress files with a failure at index 8 under --exit-on-first-failure — pre-fix the post-cancel file count wobbled (jobs=4: 9–12; jobs=8: 10–16), post-fix it pins at the analytic minimum 8 + jobs across 25/25 runs per job count. Two new threading regressions, one bounding executed files so a missed cancel blows the assert.
• Audited the rest of the round-2 layer's cross-thread surface: no second instance. Two pre-existing best-effort watchdog fields (FLastActivityNs, FCurrentFileIndex) are flagged, not silently widened into scope.
• Minor: end-of-input docs are platform-neutral (Ctrl-D / Ctrl-Z+Enter), matching the runtime strings, plus a stale code comment.

Stack #1083 layer 26.

Testing

• Verified in end-to-end tests — full suite 12,087/12,087 both modes; all 66 Pascal test programs exit 0 including Goccia.Threading.Test (14/14); CLI harness, differential harness (0 divergences), format, all doc validators exit 0
• Updated documentation — cli-conventions.md end-of-input guidance
• Optional: native Pascal tests — flag state-machine test + Pool CancelOnError bounds work across workers (8 workers, 2000 files, 10 repeats)
• Optional: benchmarks — n/a (lock is on the per-file boundary, not a hot path)

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
gocciascript-homepage	Ready	Preview	Aug 7, 2026 2:31pm

[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.

[coderabbitai]

📝 Walkthrough

Walkthrough

The thread pool now uses a synchronized shared cancellation flag. It resets the flag per run, cancels on errors, and preserves it for abandoned workers. New tests verify lifecycle and bounded cancellation. CLI documentation now describes platform-specific end-of-input keys.

Changes

Thread-pool cancellation

Layer / File(s)	Summary
Shared cancellation flag contract source/units/Goccia.Threading.pas	Added TGocciaCancellationFlag and changed workers and the pool to use its synchronized state.
Pool and worker cancellation flow source/units/Goccia.Threading.pas	Workers check the shared flag. Runs reset it. Explicit cancellation and CancelOnError invoke Cancel. Abandoned workers retain safe access to the flag.
Cancellation lifecycle and bounded-work tests source/units/Goccia.Threading.Test.pas	Added tests for flag transitions and bounded callback execution across repeated multi-worker runs.

Platform-specific CLI input documentation

Layer / File(s)	Summary
End-of-input key documentation docs/contributing/cli-conventions.md, source/app/Goccia.CLI.Application.pas	Updated documentation and comments to refer to platform-specific EndOfInputKeys values instead of only Ctrl-D.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ThreadPool
  participant Worker
  participant CancellationFlag
  participant WorkerCallback
  ThreadPool->>CancellationFlag: Reset before RunAll
  ThreadPool->>Worker: Start with shared flag
  Worker->>WorkerCallback: Process file
  WorkerCallback-->>Worker: Return error for fail.js
  Worker->>CancellationFlag: Cancel on error
  Worker->>CancellationFlag: Check IsCancelled
  ThreadPool->>CancellationFlag: Read Cancelled state

Loading

Suggested labels: bug, internal

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description includes a complete Summary and Testing section, documents implementation constraints, links issue context, and reports test and documentation results.
Title check	✅ Passed	The title clearly and concisely describes the main change: protecting the thread pool cancellation flag with synchronization.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[frostney]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@source/units/Goccia.Threading.pas`:
- Around line 641-649: Update the RunAll lifecycle around FCancelFlag and
FCancelFlagLeaked so an abandoned run’s flag is never reused: before dispatching
a later run, create and own a new TGocciaCancellationFlag when FCancelFlagLeaked
is set, then clear the leak marker after replacement. Preserve the abandoned
flag for existing workers, and add a regression test covering a stalled worker
that resumes and fails during a subsequent run without cancelling that later
run.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6e5435ef-af4e-4096-82a9-5c2ecfb2751f

📥 Commits

Reviewing files that changed from the base of the PR and between b16f39a and 659c6b1.

📒 Files selected for processing (4)

• docs/contributing/cli-conventions.md
• source/app/Goccia.CLI.Application.pas
• source/units/Goccia.Threading.Test.pas
• source/units/Goccia.Threading.pas