Make memory guardrail restart verdict reachable via chronological params

[tomerweller]

Closes #3846

Summary

The monitor-tick memory guardrail's restart verdict was unreachable: eval_memory_guardrail's heap parameters were ordered (prev, curr, prev2), but both live monitor-tick SKILL.md call sites pass the three heap snapshots chronologically (HEAP_PREV2_MB HEAP_PREV_MB HEAP_CURR_MB). This bound the oldest snapshot to heap_prev_mb and the newest to heap_prev2_mb, so the second leak-evidence delta (heap_prev - heap_prev2) was computed oldest-minus-newest — negative for any rising heap — and the -gt 500 predicate was never true. On a genuine OOM trajectory the monitor would report report-high-mem forever and never escalate to a controlled restart.

Fix (Option B from the issue): reorder the function's three heap local bindings to chronological (prev2, prev, curr) so the signature matches what the consumers already pass. The delta arithmetic is byte-identical (it references variable names), so after the reorder both growth deltas are positive under a rising heap and the restart tier fires when the RSS/avail thresholds are also met. Thresholds and the report-only arm are untouched. No SKILL.md edits needed.

Plan reference

Converged Plan comment

Test plan

• bash scripts/test-monitor-skill-snippets.sh — 488/488 pass (exit 0)
• Existing six direct-call cases preserved (heap args transposed to chronological order; verdicts unchanged)
• cargo fmt/clippy/test n/a — scripts-only (bash) change, no Rust touched

Regression test

• Test: scripts/test-monitor-skill-snippets.sh — new consumer-side case "eval_memory_guardrail: SKILL.md documented call order fires restart on +600/+600" (runs against both .claude/ and .agents/ SKILL.md copies)
• Pre-fix: committed as f9d4320 — verified FAILED with: got report-high-mem (expected restart)
• Post-fix: verified PASSES after 30db904

The regression is deliberately consumer-side: it greps the live two-line invocation out of each SKILL.md and evals it, because a direct-call test structurally cannot catch a SKILL.md/function argument-order divergence (the #3844 defect class).

Deviations from plan

None.

🤖 Generated with Claude Code

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: Correct, minimal Option-B fix — the function signature is reordered to chronological (prev2, prev, curr) so it matches both SKILL.md copies, both leak-evidence deltas are now positive on a growing heap, and a new consumer-side regression test provably fails pre-fix and passes post-fix.

Full review

Cycle 1 (no prior ## 🔍 Reviewer: Correctness comment on this PR) — complete class-labeled change-list.

Linked issue kind: bug-fix. Test-verification gate → requires a regression test that provably fails at the pre-fix commit.

Verified against origin/do/issue-3846 merged with current origin/main (main is an ancestor of the PR head — clean, up to date).

• correctness — signature/consumer alignment (satisfied): eval_memory_guardrail now binds $4→heap_prev2_mb, $5→heap_prev_mb, $6→heap_curr_mb. Both live callers — .claude/skills/monitor-tick/SKILL.md:1039-1040 and .agents/skills/monitor-tick/SKILL.md:902-903 — pass "$HEAP_PREV2_MB" "$HEAP_PREV_MB" "$HEAP_CURR_MB", which now aligns chronologically. The predicate (curr-prev)>500 && (prev-prev2)>500 yields two positive deltas on a growing heap, so the restart arm is reachable.
• correctness — no other callers (satisfied): repo-wide grep shows the only invocations are the two SKILL.md copies (plus prose mentions). No positional caller is silently broken by the reorder.
• test-coverage — regression test provably fails pre-fix (satisfied): the new block extracts the actual two-line eval_memory_guardrail "$RSS_MB" ... invocation from BOTH SKILL.md copies via awk and evals it with the documented +600/+600 trajectory (prev2=16400 prev=17000 curr=17600, RSS 47 GB / avail 7 GB / 61 GB host), asserting restart. I reproduced the pre-fix function locally: under the SKILL.md order it returns report-high-mem, not restart — so this test fails on main and passes post-fix. This closes exactly the consumer-side gap the six prior direct-call tests could not see (the monitor-tick: check (7) startup-peak phase grep never matches — tracing quotes the field, so phase=[a-z_-]+ silently yields empty #3844 defect class).
• test-coverage — both skill copies covered (satisfied): the loop iterates .claude and .agents copies, so drift in either is caught. TAP_PLAN bumped 486→488 (+2), matching the two new assertions.
• correctness — flat-heap cases unaffected (satisfied): the three updated numeric call sites are the growth cases; the flat-heap sites (17000 17000 17000) are order-independent and correctly left as-is.
• readability (satisfied): the header comment and the SKILL.md-order comment in the test are updated to the new order; the eval-safety note (reset verdict before eval, don't env-prefix the builtin) is sound.

CI signal: Monitor-Tick Skill Snippets and Script self-tests both green on the head, consistent with the above.

No blocking concerns.

[tomerweller]

🔍 Reviewer: Risk

Verdict: APPROVE

Summary: Low-risk, well-scoped fix confined to monitor bash scripts (no crate / observable-surface code). It restores a previously-dead operational safety arm (controlled restart before OOM-kill) rather than changing any live behavior that has been firing, and it is guarded by a new consumer-side regression test.

Full review

Cycle 1 (no prior ## 🔍 Reviewer: Risk comment on this PR) — complete class-labeled change-list.

Reviewed origin/do/issue-3846 against current origin/main (main is ancestor; up to date). Files touched: scripts/lib/monitor-decisions.sh, scripts/test-monitor-skill-snippets.sh — both operational monitor tooling. No parity-critical path (crates/scp|herder|ledger|tx|overlay), no observable/interop surface (hashes, tx result/meta XDR, wire bytes, history archive, HTTP/RPC/CLI, crypto). Parity lens N/A; risk lens applies.

• regression-risk (low): the change reorders a pure decision function's params and its two live call sites. The restart arm has never been eligible on the production node (28 GB headroom per the issue), and the change only makes a currently-dead arm reachable under genuine memory pressure — it cannot regress any behavior that was previously firing. Flat-heap and report-only paths are unchanged.
• operational (positive): on a no-swap validator, this is the difference between a planned ~2-min reconnect restart and an OOM-kill. Restoring the arm is the safer operational posture.
• behavioral-change scope (acceptable): the only new runtime behavior is that a genuine leak trajectory (two consecutive >500 MB heap_components_mb deltas) at RSS>0.75·RAM and avail<0.12·RAM now escalates to restart. That is the documented, intended monitor-tick: [needs-decision] recalibrate memory guardrails + surface startup peak for 32 GB validator host #3227 design. The secondary question the issue raises — whether heap_components_mb is the right leak signal at all given unaccounted-arena growth dominates on this node — is explicitly out of scope for this fix and tracked with 42h soak answers #3237's deferred question: the unaccounted climb is NOT an unbounded leak (it plateaus and reverses) — but heap_components tracks only 2.5% of a 10 GB anon swing #3845; not a blocker.
• self-modifying (N/A): paths do not match the pipeline self-mod set (scripts/lib/monitor-decisions.sh is not one of review-pr-merge/review-pr-verdicts/agent-worktree-contract/pipeline-anomaly-log; the test file and monitor-tick SKILL.md are not gated pipeline machinery). No extra-scrutiny gate required.
• security / api-shape: none — internal bash helper, no external interface.

No blocking concerns.

[tomerweller]

Review: Bounce-Back Cycle 1

Reason: CI failed (unrelated, will rebase)

Reviewer A (Correctness): APPROVE — Option-B signature reorder aligns the function with both SKILL.md copies; new consumer-side regression test provably fails pre-fix and passes post-fix.
Reviewer B (Risk): APPROVE — low-risk, monitor-script-only change that restores a previously-dead operational safety arm; no observable/parity surface touched.
External reviewers: none.
CI: red

Both agent reviewers APPROVE and there are no external change requests, but CI is red on two Quickstart integration probes:

• test (local, rpc, test_stellar_rpc_up.go test_stellar_rpc_healthy.go test_friendbot.go)
• test (local, core,rpc,horizon, test_core.go test_horizon_up.go test_horizon_core_up.go ...)

These failures are not diff-attributable. The PR touches only scripts/lib/monitor-decisions.sh and scripts/test-monitor-skill-snippets.sh (monitor bash tooling). The failing jobs are Quickstart Docker-image integration probes; the relevant excerpt is a friendbot probe timeout:

Command: timeout -k 30s 480s go run quickstart/tests/test_friendbot.go
=== Failed (exit 124), not retryable ===
##[error]Process completed with exit code 1.

Exit 124 is a wall-clock timeout of the friendbot/RPC/horizon services — infrastructure flakiness independent of the change. The monitor-specific checks (Monitor-Tick Skill Snippets, Script self-tests) are green.

Routing back to ready-for-doing for /do Mode B to rebase on origin/main and re-trigger CI. If Quickstart is still red after rebase, the next /review-pr will mark this blocked.

[tomerweller]

Do: Re-review (CI-only bounce)

Both agent reviewers already APPROVE'd; the sole blocker on cycle 1 was two unrelated flaky Go quickstart integration jobs (test (local, core,rpc,horizon …) and test (local, rpc …)) — this PR touches only scripts/lib/monitor-decisions.sh and scripts/test-monitor-skill-snippets.sh, no Go/quickstart code.

Actions taken:

• Rebased do/issue-3846 onto latest origin/main — no-op, branch is already based directly on current main (129190c); no new commit, SHA unchanged at 30db904.
• Re-verified locally: test-monitor-skill-snippets.sh → 488/488 ok, 0 failures, including the two new consumer-side regression cases (SKILL.md documented call order fires restart on +600/+600 for both the .claude and .agents copies).
• Re-triggered the two failed CI jobs via gh run rerun --failed (now in progress).

Ready for re-review once CI goes green.

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: Correct, minimal fix — reorders eval_memory_guardrail's three heap local bindings to chronological (prev2, prev, curr) so the signature matches what both live SKILL.md call sites already pass. Delta arithmetic is byte-identical; the restart tier now becomes reachable under a rising heap.

Full review

Cycle 1 (no prior ## 🔍 Reviewer: Correctness comment) — complete class-labeled change-list.

Root-cause verified [correctness]: With the old binding ($4→prev, $5→curr, $6→prev2) and the SKILL.md invocation order (HEAP_PREV2_MB HEAP_PREV_MB HEAP_CURR_MB), the second leak-evidence delta heap_prev_mb - heap_prev2_mb evaluated to oldest - newest = negative for any rising heap, so -gt 500 was never true and restart was unreachable. Confirmed by tracing scripts/lib/monitor-decisions.sh:627-628. The reorder makes both deltas newest-middle and middle-oldest, both positive under growth.

All call sites checked [regression-risk]: grepped the repo — the only invocations are the two live SKILL.md sites (.claude/ and .agents/ monitor-tick/SKILL.md), both passing "$HEAP_PREV2_MB" "$HEAP_PREV_MB" "$HEAP_CURR_MB" (chronological). No caller passed the old (prev,curr,prev2) order, so the reorder breaks nothing. The six direct-call test cases were transposed to match; their expected verdicts are unchanged.

Test-verification gate (bug-fix) PASSED [test-coverage]: The PR adds a consumer-side regression that greps the live two-line invocation out of each SKILL.md and evals it — structurally the only way to catch a SKILL.md/function argument-order divergence (the #3844 defect class), since a direct-call test passes args in the function's own order and cannot detect the mismatch. I verified independently:

• Post-fix (HEAD 30db904 merged with current origin/main): bash scripts/test-monitor-skill-snippets.sh → 488/488, exit 0; new cases 439/440 pass for both SKILL.md copies.
• Pre-fix (regression test from f9d4320 against pre-fix monitor-decisions.sh): cases 439/440 → not ok, exit 1. The test provably fails at the pre-fix commit.

Readability/error-handling [none]: The awk extraction captures both continuation lines robustly; the test resets MEMORY_GUARDRAIL_VERDICT="" immediately before each eval so a prior value can't mask a regression. Comments are accurate. Nothing to flag.

No blocking concerns.

[tomerweller]

🔍 Reviewer: Risk

Verdict: APPROVE

Summary: Scripts-only bash change to a monitor-tick decision helper. No observable/interop surface, no Rust, no data format touched. Behavioral change is strictly a fix (makes the restart tier reachable); thresholds and the report-only arm are untouched. Low risk.

Full review

Cycle 1 (no prior ## 🔍 Reviewer: Risk comment) — complete class-labeled change-list.

Blast radius [regression-risk]: Only scripts/lib/monitor-decisions.sh (4-line binding reorder) and its test file change. eval_memory_guardrail is an internal operational guardrail for the mainnet monitor loop — not part of the parity/interop surface (no hashes, XDR, wire bytes, archive format, or RPC/CLI contract). Per docs/PARITY.md this is squarely internal.

Behavioral delta [correctness]: The only runtime effect is that a genuine OOM trajectory (RSS>0.75·RAM AND avail<0.12·RAM AND two consecutive heap deltas >500 MB) now yields restart instead of being pinned at report-high-mem forever. That is the intended repair. The report-only threshold (>0.65·RAM) and the none path are unchanged; I confirmed the "no false-fire on 61 GB" case (447/…) still returns none.

Operational safety [ops]: Making restart reachable is the safer direction — the prior bug meant an actually-leaking node would never self-heal. Restart remains gated behind the conjunction of RSS, avail, and dual-delta leak evidence, so it will not fire on transient spikes.

CI note [ci-failure]: The two red checks are Quickstart test (local, …) Docker integration jobs (friendbot/rpc/horizon) that timed out waiting for external services (exit 124) — unrelated to a bash-script change. All directly-relevant checks (Monitor-Tick Skill Snippets, Script self-tests, Project-Loop Skill Snippets, Clippy, Test, Format) are green. This is a non-blocking, infra-flaky failure.

No blocking concerns.

[tomerweller]

Review: Bounce-Back Cycle 1

Reason: CI failed (unrelated, will rebase)

Reviewer A: APPROVE — correct, minimal fix; regression test provably fails at pre-fix commit and passes post-fix (488/488).
Reviewer B: APPROVE — scripts-only internal guardrail change; no observable/parity surface; makes restart reachable (safe direction).
CI: red

Both agent reviewers APPROVE and the fix is verified locally (post-fix 488/488 exit 0; pre-fix regression cases 439/440 fail exit 1). The only red checks are unrelated flaky Quickstart Docker integration jobs:

• test (local, rpc, test_stellar_rpc_up.go test_stellar_rpc_healthy.go test_friendbot.go) — timed out (exit 124) waiting for Friendbot to become available.
• test (local, core,rpc,horizon, …) — same Quickstart external-service startup timeout.

These spin up external Docker services (friendbot/rpc/horizon) and are unrelated to this 4-line bash reorder. All directly-relevant checks are green: Monitor-Tick Skill Snippets, Script self-tests, Project-Loop Skill Snippets, Clippy, Test, Format.

Routing back to ready-for-doing for /do Mode B to rebase on origin/main and re-trigger CI. The PR head is already current with origin/main, so the rebase is effectively a no-op push to re-run the flaky Quickstart jobs. If CI is still red after the retry, the next /review-pr will mark blocked.

[tomerweller]

Do: Re-review (CI-only bounce, cycle 2)

No code change required. Both agent reviewers already APPROVE, and there are no inline or actionable review comments since the last push. The sole blocker remains the two flaky Quickstart Docker integration jobs.

Actions taken:

• Rebased do/issue-3846 onto origin/main — no-op. origin/main (129190c) is an ancestor of the PR head (30db904); the branch is already current. No new commit, SHA unchanged.
• Re-verified locally: bash scripts/test-monitor-skill-snippets.sh → 488/488 ok, 0 failures, exit 0, including the two new consumer-side regression cases (439/440: SKILL.md documented call order fires restart on +600/+600) for both the .claude and .agents copies.
• Re-triggered the two failed Quickstart jobs (gh run rerun --failed, now in progress).

Failure diagnosis (unchanged, confirmed from the fresh 09:34 re-run logs): both red jobs are the Quickstart test (local, …) probes timing out while Waiting for Friendbot to be available, ending === Failed (exit 124), not retryable === — a wall-clock timeout of external Docker services (friendbot/rpc/horizon), not diff-attributable. This PR touches only scripts/lib/monitor-decisions.sh and scripts/test-monitor-skill-snippets.sh (monitor bash tooling); no Go/quickstart code. All directly-relevant checks are green (Monitor-Tick Skill Snippets, Script self-tests, Project-Loop Skill Snippets, Clippy, Test, Format, CodeQL).

Ready for re-review once CI settles.

[tomerweller]

Addressed feedback (CI-only bounce; no code change needed). Re-triggered flaky Quickstart jobs. Ready for re-review.

[tomerweller]

Review: Bounce-Back Cycle 3

Reason: CI failed (unrelated — flaky Quickstart Docker integration timeout, will re-run)

Reviewer A (Correctness): APPROVE — clean chronological (prev2, prev, curr) binding reorder matching both live SKILL.md call sites; consumer-side regression test provably fails pre-fix.
Reviewer B (Risk): APPROVE — scripts-only bash change to an internal monitor-tick guardrail; no observable/interop surface touched; makes the restart tier reachable (the intended repair).
External reviewers: tomerweller: COMMENTED (neutral, non-blocking).
CI: red (unrelated)

Both agent reviewers APPROVE on the current head (do/issue-3846 @ 2026-08-27T08:11:12Z). The only red signal is the Quickstart workflow: two test (local, …) Docker jobs (friendbot/rpc/horizon) failed with exit 124 — the services never became available ("Waiting for Friendbot to be available" looped until the probe budget expired). This is infra flakiness, not diff-attributable: the PR changes only scripts/lib/monitor-decisions.sh (a bash decision helper) and its test script. All directly-relevant checks are green (Monitor-Tick Skill Snippets, Script self-tests, Project-Loop Skill Snippets, Clippy, Test, Format, CodeQL).

origin/main (129190c, 2026-08-25) is not ahead of this branch, so a plain rebase is a no-op. To clear the flaky red, re-trigger the Quickstart workflow (e.g. gh run rerun <id> --failed, or push a trivial no-op commit to re-run CI).

Escalation note: this is the 3rd bounce on the current (unchanged) code state, all for the same unrelated flaky-CI reason. Per the head-scoped cap, if the flaky Quickstart recurs on the next review without a fresh CI pass, the PR should be blocked for operator attention (re-run the flaky job, or treat Quickstart as non-required for a scripts-only change).

Routing back to ready-for-doing for /do Mode B.

[tomerweller]

Do: CI Re-trigger (Mode B, bounce cycle 3)

No code changes — both agent reviewers APPROVE the current head and there are zero actionable inline review comments. The sole red signal was the flaky Quickstart Docker integration jobs (test (local, …), exit 124: services never became available), which are not diff-attributable — this PR only touches scripts/lib/monitor-decisions.sh and its test script.

Action taken: re-ran the failed Quickstart jobs via gh run rerun 33052953955 --failed. Both jobs are now in progress. All directly-relevant checks (Monitor-Tick Skill Snippets, Script self-tests, Clippy, Test, Format, CodeQL) are already green.

Ready for re-review once CI settles.

[tomerweller]

🔍 Reviewer: Correctness

Verdict: APPROVE

Summary: Re-review on unchanged head 30db904 (same code state as the cycle-1 approval). Correct, minimal Option-B fix: eval_memory_guardrail's three heap local bindings are reordered to chronological (prev2, prev, curr) so the signature matches what both live SKILL.md call sites already pass. The restart tier is now reachable on a rising heap.

Full review

Cycle N≥2 — prior ## 🔍 Reviewer: Correctness verdicts exist (09:04, 10:06), both APPROVE. Classes previously raised: correctness, regression-risk, test-coverage, readability. No new class discovered; the code has not changed since those approvals (head still 30db904).

• correctness (satisfied): $4→heap_prev2_mb, $5→heap_prev_mb, $6→heap_curr_mb. The predicate (curr-prev)>500 && (prev-prev2)>500 yields two positive deltas on a growing heap; the previously-dead restart arm is reachable. Delta arithmetic references variable names, so it is byte-identical after the reorder.
• regression-risk (satisfied): the only callers are the two live SKILL.md copies (.claude/ + .agents/), both passing HEAP_PREV2_MB HEAP_PREV_MB HEAP_CURR_MB. No positional caller is broken. The six direct-call test cases were transposed to chronological order; expected verdicts unchanged. Flat-heap cases are order-independent and correctly left as-is.
• test-coverage (satisfied): the new consumer-side regression greps the live two-line invocation out of both SKILL.md copies and evals it under the documented +600/+600 trajectory — the only structural way to catch a SKILL.md/function argument-order divergence (monitor-tick: check (7) startup-peak phase grep never matches — tracing quotes the field, so phase=[a-z_-]+ silently yields empty #3844 defect class). Provably fails pre-fix (f9d4320), passes post-fix. TAP_PLAN 486→488 (+2), matching the two new assertions.

No blocking concerns. Only red signal is unrelated Quickstart CI (see the review decision comment).

[tomerweller]

🔍 Reviewer: Risk

Verdict: APPROVE

Summary: Re-review on unchanged head 30db904. Scripts-only bash change to a monitor-tick decision helper — no crate code, no observable/interop surface, not pipeline self-modifying machinery. Restores a previously-dead operational safety arm; thresholds and the report-only arm are untouched. Low risk.

Full review

Cycle N≥2 — prior ## 🔍 Reviewer: Risk verdicts exist (09:04, 10:06), both APPROVE. Classes previously raised: regression-risk, operational, behavioral-change scope, self-modifying (N/A), security/api-shape. No new class discovered.

• regression-risk (low): reorders a pure decision function's params and updates its two live call sites + six direct-call tests. The restart arm has never been eligible on the production node (~28 GB headroom per monitor-tick: memory guardrail's restart verdict is unreachable — SKILL.md passes heap snapshots chronologically but eval_memory_guardrail's params are (prev, curr, prev2), inverting one delta #3846), so this cannot regress any behavior that was firing — it only makes a dead arm reachable under genuine pressure.
• parity (N/A): files are scripts/lib/monitor-decisions.sh + scripts/test-monitor-skill-snippets.sh. No parity-critical crate (scp|herder|ledger|tx|overlay), no hashes/XDR/wire-bytes/archive/RPC/CLI/crypto surface. Per docs/PARITY.md this is purely internal.
• self-modifying (N/A): paths do not match the pipeline self-mod set (review-pr-merge/review-pr-verdicts/agent-worktree-contract/pipeline-anomaly-log, skill dirs, or .github/skills/shared/scripts/). No extra-scrutiny gate.
• operational (positive): on a no-swap validator this is the difference between a planned ~2-min reconnect restart and an OOM-kill.

No blocking concerns. Only red signal is unrelated Quickstart CI.

[tomerweller]

Review: Bounce-Back Cycle 2

Reason: CI failed (unrelated, will rebase)

Reviewer A (Correctness): APPROVE — Option-B signature reorder aligns the function with both SKILL.md copies; consumer-side regression provably fails pre-fix and passes post-fix.
Reviewer B (Risk): APPROVE — scripts-only monitor-tooling change; no observable/parity surface; restores a previously-dead operational safety arm.
External reviewers: none (the only PR review is the author's own COMMENTED note).
CI: red

Both agent reviewers APPROVE and there are no external change requests, but CI is red on two Quickstart integration probes:

• test (local, rpc, test_stellar_rpc_up.go test_stellar_rpc_healthy.go test_friendbot.go)
• test (local, core,rpc,horizon, test_core.go test_horizon_up.go test_horizon_core_up.go ...)

These are not diff-attributable. The PR touches only scripts/lib/monitor-decisions.sh and scripts/test-monitor-skill-snippets.sh (monitor bash tooling); the failing jobs are Quickstart Docker-image integration probes. RPC came up healthy, then the friendbot probe hit a wall-clock timeout:

Command: timeout -k 30s 1200s go run quickstart/tests/test_friendbot.go
=== Failed (exit 124), not retryable ===
##[error]Process completed with exit code 1.

Exit 124 is a friendbot/RPC service timeout — infrastructure flakiness independent of a bash param reorder. The monitor-specific checks (Monitor-Tick Skill Snippets, Script self-tests) are green.

Note: the cycle-1 bounce was resolved by a CI-only re-trigger (head unchanged at 30db904); the re-triggered Quickstart run failed again with the same friendbot timeout. /do Mode B should this time rebase on the latest origin/main (not just re-trigger) to pick up any Quickstart infra fixes, then re-run CI. If Quickstart is still red after a genuine rebase, the next /review-pr will mark this blocked for operator investigation of the flaky probe.

Routing back to ready-for-doing.

[tomerweller]

Do: CI Re-trigger (Mode B, full Quickstart re-run)

No code changes — both agent reviewers APPROVE the current head (30db904) and there are zero actionable inline review comments since the last push. origin/main (129190c) is already an ancestor of the branch head, so a rebase is a verified no-op.

The sole red signal remains the two flaky Quickstart Docker integration jobs (test (local, rpc …) and test (local, core,rpc,horizon …)), both exit 124 — external services (friendbot/rpc/horizon) never became available before the wall-clock budget. Not diff-attributable: this PR only touches scripts/lib/monitor-decisions.sh and scripts/test-monitor-skill-snippets.sh (monitor bash tooling).

Action taken: Prior re-runs of just the failed jobs kept timing out in the same run context, so I triggered a full re-run of the Quickstart workflow (run 33052953955) to get fresh runners and fresh service containers — the more likely path to clear a startup-timeout flake than re-running the same two jobs. All directly-relevant checks (Monitor-Tick Skill Snippets, Script self-tests, Project-Loop Skill Snippets, Clippy, Test, Format, CodeQL) are already green.

Ready for re-review once CI settles. Per the head-scoped bounce cap, if Quickstart is still red on the next /review-pr the PR should be blocked for operator attention (re-run the flaky probe, or treat Quickstart as non-required for a scripts-only change).

[tomerweller]

Full Quickstart workflow re-run triggered (fresh containers) to clear the flaky friendbot/rpc/horizon startup timeouts. No code change — rebase is a no-op and both reviewers already APPROVE. Ready for re-review.

[tomerweller]

Review: Bounce-Back Cycle 2

Reason: CI failed (unrelated, will rebase)

Reviewer A (Correctness): APPROVE — 4-line heap-param reorder to chronological (prev2, prev, curr) matches both SKILL.md call sites; consumer-side regression test added.
Reviewer B (Risk): APPROVE — scripts-only change; delta arithmetic references variable names so it is byte-identical after the reorder. No observable/interop surface touched.
External reviewers: tomerweller: COMMENTED (non-blocking; re-triggered flaky Quickstart jobs)
CI: red (unrelated — flaky friendbot startup timeout)

The only red checks are the Quickstart shards test (local, rpc, ...) and test (local, core,rpc,horizon, ...), both failing on the well-known flaky friendbot startup timeout:

[test_friendbot] Waiting for Friendbot to be available   (repeated ~4 min)
Diagnostics saved to /tmp/quickstart-diagnostics/local-rpc-friendbot/attempt-1
=== Failed (exit 124), not retryable ===
##[error]Process completed with exit code 1.

This is a docker/infra startup flake in the Quickstart harness, entirely unrelated to this PR's diff, which touches only scripts/lib/monitor-decisions.sh (a 4-line bash arg reorder) and scripts/test-monitor-skill-snippets.sh. Both agent reviewers APPROVE the current head.

Manual job re-triggers (Quickstart attempt 6) keep hitting the same friendbot flake. Routing back to ready-for-doing so /do Mode B performs a proper rebase onto the latest origin/main (which may already carry a Quickstart-flake mitigation) and re-runs CI from a fresh commit. If CI is still red on the friendbot flake after a clean rebase, the next /review-pr will mark this blocked for operator attention.

[tomerweller]

Do: Blocked — CI-only bounce loop, no actionable fix

Mode B ran with zero actionable inline review comments and no code change to make. Both agent reviewers (Correctness + Risk) APPROVE the current head; the only external review is a neutral non-blocking COMMENT. I am not performing another CI re-trigger, because doing so is what has kept this PR in an infinite loop.

Why this is a loop, not progress:

• Head SHA 30db904 is unchanged across the entire history — every "fix" cycle has been a no-op.
• This code state has been bounced back 5 times (## Review: Bounce-Back ×5) with 4 /do no-op CI re-triggers (## Do: Re-review / CI Re-trigger ×4). There are no ## Review: Reset markers, so the head-scoped bounce cap (blocks after 3 bounce-backs on the same code state) has been exceeded nearly 2×.
• Every bounce has the same non-diff-attributable cause: the two Quickstart Docker integration shards test (local, core,rpc,horizon …) and test (local, rpc …) fail with exit 124 (friendbot/rpc/horizon services never become available within the wall-clock budget). This PR touches only scripts/lib/monitor-decisions.sh (a 4-line bash param reorder) and scripts/test-monitor-skill-snippets.sh — no Go/quickstart code.
• Re-triggering the failed jobs and even a full workflow re-run (per the prior ## Do notes) have already been tried and keep hitting the identical friendbot startup flake.

All directly-relevant checks are green: Monitor-Tick Skill Snippets, Script self-tests, Project-Loop Skill Snippets, Clippy, Test, Format, CodeQL, Build, Doctest.

Operator action needed (out of /do's scope):

1. Treat Quickstart as non-required for scripts-only diffs, or admin-merge this approved PR (both agent reviewers APPROVE, CI red is infra-only); or
2. Fix the flaky Quickstart friendbot startup probe at the harness level.

Moving to blocked for operator attention instead of perpetuating the review↔do loop.