Give crontab watchdogs a real tick post-condition

[tomerweller]

Closes #3780

Summary

The crontab watchdogs (monitor-watchdog.sh, project-loop-watchdog.sh) recorded exit=$? from claude -p as the tick result, but that is the CLI's exit status, not the tick's. A refusal to run — org monthly spend limit, auth failure — exits 0 and was indistinguishable from a completed tick. On 2026-07-28/29 this produced ~24h of dark mainnet monitoring logged as 49 "successful" ticks, and because the refire-cooldown MARKER was written before the launch, each 1-second no-op burned the full 30-min cooldown.

This adds a real post-condition keyed on the only true success signal — a new tick-history.jsonl row:

• New source-safe lib scripts/lib/watchdog-postcondition.sh: watchdog_hist_count and a pure watchdog_classify_outcome returning success (row advanced, duration ≥ floor), suspect-fast (row advanced but under the floor), noop-refusal (no row + known refusal regex), or noop-empty (no row, no recognized reason).
• Both watchdogs capture the row count + duration around the launch, log a distinct outcome=<class> dur=Ns rows=N line (preserving the captured CLI output in the log), write the cooldown MARKER only on success (a no-op now retries at the next */15 slot), and emit a greppable ESCALATION line after ESCALATE_AFTER (default 3) consecutive failed launches.
• Path/binary constants are env-overridable with the current hardcoded values as defaults, so production behavior is unchanged and tests can point at a fixture.
• The canonical scripts/monitor-watchdog.sh is now tracked in-repo (previously only a live crontab copy; its own header claimed a canonical repo copy that did not exist).

Plan reference

Converged Plan comment

Test plan

• bash scripts/test-watchdog-postcondition.sh — 15/15 pass (was 13/15 failing pre-fix)
• bash scripts/test-project-loop-skill-snippets.sh — unaffected, passes
• bash -n clean on both watchdogs and the lib
• cargo fmt --check clean (no Rust files changed)
• .github/workflows/ci.yml valid YAML; new watchdog-postcondition job wired

No Rust crate is touched, so cargo clippy/cargo test carry no signal for this change and were not run.

Regression test (kind: bug-fix)

• Test: scripts/test-watchdog-postcondition.sh (test_classify_spend_limit_noop, test_e2e_spend_limit_does_not_burn_cooldown, plus success/empty/suspect-fast + project-loop e2e branches)
• Pre-fix: committed as 581ecd4 — verified FAILED (13 of 15 assertions: shared lib + canonical monitor-watchdog.sh absent; project-loop-watchdog.sh logged bare exit=$rc with a pre-tick MARKER write, no outcome=noop-refusal).
• Post-fix: verified PASSES (15/15) after 6c33e2f.

Parity considerations

n/a — operator crontab tooling only. No observable/interop surface (ledger/bucket hashes, result/meta XDR, SCP/overlay wire, history-archive format, HTTP/RPC/CLI contract, crypto output) is touched. (Critic B: APPROVE.)

Deviations from plan

• Added a project-loop-watchdog e2e (spend-limit no-op via a local bare-repo REMOTE, offline) beyond the plan's six named tests, so the second modified script's fix is guarded directly rather than only through the shared lib. Per the plan's Critic-A note, the clone path is exercised against a local bare repo under the test's tempdir.

Operator action (post-merge, not code)

1. Re-sync the live crontab copies /home/tomer/data/monitor-watchdog.sh and /home/tomer/data/project-loop-watchdog.sh from the fixed canonical scripts, and copy scripts/lib/watchdog-postcondition.sh alongside them (the live copies source it via $SCRIPT_DIR/lib/…, overridable with WATCHDOG_LIB).
2. The underlying org monthly spend limit (the incident's root cause) is not addressed by code — raise or alert-on-approach separately.
3. Follow-up: a genuine human-facing escalation channel (GH issue / push notification / overlay broadcast) is out of scope here — this PR ships only the self-contained greppable ESCALATION line (per Critic C).

🤖 Generated with Claude Code

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: The fix correctly replaces the meaningless exit=$? success signal with a real post-condition (new tick-history.jsonl row + duration floor), writes the cooldown MARKER only on genuine success, and adds a consecutive-failure escalation. The regression harness reproduces the incident and passes 15/15; verified locally on the PR head merged with current main.

Full review

Cycle 1 (no prior Correctness verdict on this PR) — complete class-labeled change-list.

Verified locally (PR head 6c33e2f, already current with origin/main):

• bash -n clean on all four scripts.
• scripts/test-watchdog-postcondition.sh → 1..15, all ok, exit 0. Matches the CI Watchdog Post-Condition job (green).

Correctness — no blocking findings:

• watchdog_classify_outcome logic is sound: post > pre gates row-advance, then the duration floor splits success vs suspect-fast; otherwise refusal-regex splits noop-refusal vs noop-empty. Pre/post both use wc -l, so any trailing-newline off-by-one cancels in the delta.
• Removing the pre-tick MARKER write is safe against double-dispatch: flock -n 9 already serializes overlapping cron instances during a long-running tick, and on success a fresh history row makes the staleness gate skip the next slot. On failure, no MARKER → retry at the next */15 — exactly the intended fix.
• test-coverage: bug-fix test-verification gate satisfied — the harness exercises the real scripts and the MARKER-write/failstreak behavior (not just the pure classifier), PR documents pre-fix 581ecd4 failing 13/15 and post-fix 6c33e2f passing 15/15, and TAP_PLAN=15 matches the 15 assertions.

Non-blocking notes (inline-class, do not gate):

• error-handling: the case "$pre$post$dur$floor" in *[!0-9]*) ;; esac guard in watchdog_classify_outcome has an empty body — it is dead code and does not actually sanitize non-numeric input. It happens to be harmless because the [ ... ] 2>/dev/null comparisons fall through to the refusal/empty branch on bad input, but the comment overstates what the line does. Consider removing it or making it real.
• regression-risk: suspect-fast increments the failstreak and can therefore contribute to a false ESCALATION if a legitimate tick ever completes under the 30 s floor. Given real ticks run p50 372 s this is low-risk, and the escalation is only a greppable log line, but worth a comment.

[tomerweller]

🔍 Reviewer: Risk

Verdict: APPROVE

Summary: Operator crontab tooling and one CI job only — no observable/interop surface (ledger/bucket hashes, tx result/meta XDR, SCP/overlay wire, history-archive format, HTTP/RPC/CLI contract, crypto output) is touched. No parity-critical crate is in the diff. Operational risk is low and net-positive: the change turns a silent 24 h dark-monitoring failure mode into a retrying, escalating one.

Full review

Cycle 1 (no prior Risk verdict on this PR) — complete class-labeled change-list.

Parity/observable surface: n/a. Changed paths are scripts/monitor-watchdog.sh, scripts/project-loop-watchdog.sh, scripts/lib/watchdog-postcondition.sh, scripts/test-watchdog-postcondition.sh, and a watchdog-postcondition job in .github/workflows/ci.yml. None of these are on the interop surface; no Rust crate is touched.

Operational risk — no blocking findings:

• The new CI job is cheap and bounded (timeout-minutes: 5, runs one shell harness) and is passing.
• Env-overridable path/binary constants default to the current hardcoded production values, so live crontab behavior is unchanged until the operator re-syncs (the PR calls this out as a required post-merge operator action).
• security: no new secret handling, no privilege change; refusal-regex is matched case-insensitively against captured stdout only.

Non-blocking notes (inline-class, do not gate):

• efficiency: dropping the pre-tick MARKER means that during a sustained refusal outage project-loop-watchdog.sh will git clone --depth 1 the repo every 15 min (previously suppressed by the 30-min cooldown). Each refusal claude -p is near-zero cost and the clone is shallow, and this is the intended trade to enable fast recovery, but it is a modest increase in churn during an outage.

CI signal: the only non-green rollup entry is test (testnet, core,horizon, …) = CANCELLED. That job hung ~55 min on the live-testnet "Run probes through wrapper" step and was timeout-cancelled; every sibling (build, local, pubnet) passed. It is testnet-infra flakiness, unrelated to a shell/CI diff — but it still leaves the CI signal non-green, which the merge gate must respect.

[tomerweller]

Review: Bounce-Back Cycle 1

Reason: CI failed (unrelated, will rebase)

Reviewer A: APPROVE — post-condition fix is correct; regression harness reproduces the incident and passes 15/15 (verified locally on PR head).
Reviewer B: APPROVE — operator crontab/CI tooling only; no observable/interop surface touched; operational risk low and net-positive.
External reviewers: none
CI: red

Both agent reviewers approve. The sole non-green CI signal is unrelated infra flakiness, not attributable to this diff:

• test (testnet, core,horizon, …) = CANCELLED (Quickstart run 32620216502). The job hung ~55 min on the live-testnet Run probes through wrapper step and was timeout-cancelled; every sibling job (build, test (local, …), test (pubnet, …)) passed. This PR changes only shell watchdog scripts and one CI job — nothing on the quickstart/testnet path — so the cancellation cannot be diff-attributable.

The merge gate still requires a green CI rollup. Routing back to ready-for-doing so /do Mode B rebases on origin/main and re-triggers CI; a fresh run should clear the flaky testnet probe. No code changes are requested. If CI is still red after rebase, the next /review-pr will mark this blocked.

Bounce-back count (head-scoped): 1/3.

[tomerweller]

Addressed bounce-back cycle 1. Ready for re-review.

Both agent reviewers (Correctness, Risk) already APPROVED with no requested code changes. The sole non-green signal was test (testnet, core,horizon, …) = CANCELLED — live-testnet infra flakiness unrelated to this shell/CI diff.

Branch is already directly on top of current origin/main (no divergence), so the rebase was a no-op and no code changed. Re-triggered the cancelled Quickstart run (32620216502) on the same head 6c33e2f to clear the flaky testnet probe. A fresh green rollup should now let the merge gate proceed.

The two non-blocking inline notes from the Correctness review (empty-body numeric guard in watchdog_classify_outcome; suspect-fast contributing to failstreak) were explicitly marked "do not gate" and are left as-is to keep the bounced PR scope fixed.

[tomerweller]

Addressed bounce-back cycle 1 (re-run). Ready for re-review.

Both agent reviewers (Correctness, Risk) already APPROVED with no requested code changes and no inline review comments. Branch is already up to date with origin/main (merge-base == current main 5ac2f16), so no rebase was possible or needed.

The sole non-green CI signal was test (testnet, core,horizon, …) = CANCELLED — live-testnet infra flakiness unrelated to this shell/CI-yaml-only diff. I re-ran the failed Quickstart jobs to clear it. All other checks are green, including the PR's own Watchdog Post-Condition regression harness (15/15 assertions pass locally and in CI).

[tomerweller]

Review: CI Stuck — Blocked (persistent unrelated testnet cancellation)

Reviewer A (Correctness): APPROVE — post-condition fix correct; regression harness reproduces the incident (15/15).
Reviewer B (Risk): APPROVE — operator crontab tooling + one CI job only; no observable/interop surface touched.
External reviewers: none.
CI: red — 1 non-green check.
Bounce count (head-scoped): 1
Status: blocked

Pattern: The only non-green rollup entry is test (testnet, core,horizon, …) = CANCELLED (Quickstart run 32620216502), timeout-cancelled at 09:43:22Z after ~55 min on the live-testnet integration path. Every sibling job is green — build, all local variants, the pubnet core/rpc/horizon test, galexie, and the PR's own new Watchdog Post-Condition regression job. The diff touches only shell scripts (scripts/monitor-watchdog.sh, scripts/project-loop-watchdog.sh, scripts/lib/watchdog-postcondition.sh, scripts/test-watchdog-postcondition.sh) and a .github/workflows/ci.yml job; none of it runs inside a live-testnet core+horizon integration test, so the cancellation is unrelated infra flakiness, not diff-attributable.

This is the escalation the cycle-1 bounce promised: cycle 1 (2026-08-23T08:18Z) bounced to ready-for-doing noting "if the testnet job is still red after a clean rebase, the next /review-pr will mark this blocked." /do Mode B took its remediation shot — branch was already on top of origin/main (no rebase possible/needed) and it re-triggered the cancelled Quickstart job — and the identical live-testnet job cancelled again. Continuing to bounce would only churn /do, which cannot fix a live-testnet infra timeout.

Operator action required: both reviewer lenses APPROVE and every diff-attributable check is green. The merge is blocked solely on the flaky/timing-out live-testnet test (testnet, core,horizon, …) Quickstart probe. Either (a) re-run that single job when testnet infra is healthy and, once green, post ## Review: Reset to let this PR re-attempt the merge gate, or (b) if the testnet probe is chronically timing out for all PRs, address the CI infra separately.