Review finding 1: last_heartbeat and TickWork::Heartbeat were gated on outcome.is_ok(). The given test fails today with left: [] / right: [Heartbeat]. Co-Authored-By: Grok 4.6 <noreply@x.ai>
139 lines
5.4 KiB
Markdown
139 lines
5.4 KiB
Markdown
# M5d-2 task 11: a failed heartbeat still advances `last_heartbeat`
|
|
|
|
**Branch:** `m5d2`. Check it: `git branch --show-current` prints `m5d2` and `git status --short`
|
|
is empty; otherwise stop. Never switch branch.
|
|
**Commit subject:** `loopd: a failed heartbeat still advances last_heartbeat`
|
|
**Log row:** the Task cell is `M5d2/11-loopd-heartbeat-failed-last`.
|
|
|
|
**First action.** Run this before any Read of crate source. Do not `ls` `crates/loopd/tests/`.
|
|
Do not Read `tests/support/`, `session.rs`, `turn.rs`, `compact.rs`, `jobs.rs`, or
|
|
`docs/implementer-log.md` (except the first 12 lines at the log step). The quoted block is
|
|
the only edit.
|
|
|
|
```sh
|
|
cp docs/plans/M5d2/files/crates/loopd/tests/scheduler_heartbeat_failed.rs crates/loopd/tests/
|
|
```
|
|
|
|
## Goal
|
|
|
|
After `run_turn` and `finish_turn` on the standing `heartbeat` session, always set
|
|
`last_heartbeat = Some(now)` and push `TickWork::Heartbeat`, including when the outcome is
|
|
`Err` (M5d-2 review finding 1; task 06's "The turn" fact). A failed pulse must not be due
|
|
again on the next 1 s tick.
|
|
|
|
An unreadable prompt still does not update `last_heartbeat`. A busy skip still does not.
|
|
Cron `Finished` / `Failed` is unchanged.
|
|
|
|
## Every place that changes
|
|
|
|
| Place | What happens there |
|
|
|---|---|
|
|
| `crates/loopd/src/scheduler.rs` | after the heartbeat turn, bookkeeping is unconditional (edit H1) |
|
|
| `crates/loopd/tests/scheduler_heartbeat_failed.rs` | new, given |
|
|
| everything else | **no change** |
|
|
|
|
## Files
|
|
|
|
- Copy, from `docs/plans/M5d2/files/` (the same path under it): the new test
|
|
`crates/loopd/tests/scheduler_heartbeat_failed.rs`.
|
|
- Modify: `crates/loopd/src/scheduler.rs`, `docs/implementer-log.md`.
|
|
- Do not touch: any other file. Do not edit `crates/loopd/tests/scheduler_heartbeat.rs`.
|
|
|
|
## Facts you need
|
|
|
|
- **H1.** In `heartbeat`, the quoted block is found exactly once. Replace it with the after
|
|
block. `finish_turn` still runs. `drop(held)` still runs. Then `last_heartbeat` and
|
|
`Heartbeat`, with no `if outcome.is_ok()`.
|
|
- **Unreadable prompt / Busy.** Those `return` paths stay as they are. They never reach H1.
|
|
- **`Reply::fixture("bad_request")`** is HTTP 400. It is not retried. The given test does
|
|
not sleep (T27).
|
|
|
|
## The edit (H1)
|
|
|
|
In `crates/loopd/src/scheduler.rs`, this block (exactly once):
|
|
|
|
```rust
|
|
let text = format!("It is {}.\n\n{file}", now.to_rfc3339());
|
|
let mut emit = |_event: &TurnEvent| {};
|
|
let outcome = run_turn(&mut session, &runtime, &text, None, &mut emit);
|
|
let outcome = compact::finish_turn(
|
|
&mut session,
|
|
&runtime,
|
|
&*ctx.memory,
|
|
&text,
|
|
None,
|
|
&mut emit,
|
|
outcome,
|
|
);
|
|
if outcome.is_ok() {
|
|
self.last_heartbeat = Some(now);
|
|
work.push(TickWork::Heartbeat);
|
|
}
|
|
drop(held);
|
|
```
|
|
|
|
becomes:
|
|
|
|
```rust
|
|
let text = format!("It is {}.\n\n{file}", now.to_rfc3339());
|
|
let mut emit = |_event: &TurnEvent| {};
|
|
let outcome = run_turn(&mut session, &runtime, &text, None, &mut emit);
|
|
let _ = compact::finish_turn(
|
|
&mut session,
|
|
&runtime,
|
|
&*ctx.memory,
|
|
&text,
|
|
None,
|
|
&mut emit,
|
|
outcome,
|
|
);
|
|
drop(held);
|
|
self.last_heartbeat = Some(now);
|
|
work.push(TickWork::Heartbeat);
|
|
```
|
|
|
|
## Steps
|
|
|
|
- [ ] **1. Copy the given test.**
|
|
|
|
```sh
|
|
cp docs/plans/M5d2/files/crates/loopd/tests/scheduler_heartbeat_failed.rs crates/loopd/tests/
|
|
```
|
|
|
|
- [ ] **2. See it fail.** `cargo test -p loopd --test scheduler_heartbeat_failed`, timeout
|
|
600000 ms. Expected: one test fails,
|
|
`a_failed_heartbeat_turn_still_advances_last_heartbeat`, with `left: []` and
|
|
`right: [Heartbeat]`. Every other loopd test target is untouched. Do not open the test
|
|
to find out why.
|
|
- [ ] **3. Make H1.** The before block is found exactly once. `cargo fmt --all`.
|
|
`cargo check -p loopd` prints no error and no warning.
|
|
- [ ] **4. See it pass.** `cargo test -p loopd --test scheduler_heartbeat_failed --test
|
|
scheduler_heartbeat`: every test passes, 1 in `scheduler_heartbeat_failed`, 6 in
|
|
`scheduler_heartbeat`.
|
|
- [ ] **5. Run the gate.** `cargo fmt --all`, then `make gate`, timeout 600000 ms. Last line
|
|
`gate: ok`. Test count **1575** (1574 before, plus 1). After `gate: ok`, do not add up
|
|
`test result:` lines.
|
|
- [ ] **6. Log and commit.** Task cell `M5d2/11-loopd-heartbeat-failed-last`. Read only the
|
|
first 12 lines of `docs/implementer-log.md`; insert the row under the header. Stage with
|
|
`git add crates/loopd/src/scheduler.rs crates/loopd/tests/scheduler_heartbeat_failed.rs
|
|
docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` says.
|
|
|
|
## Done when
|
|
|
|
- `make gate` prints `gate: ok` with 1575 tests.
|
|
- `git diff HEAD~1 --stat` lists only `crates/loopd/src/scheduler.rs`,
|
|
`crates/loopd/tests/scheduler_heartbeat_failed.rs` and the log.
|
|
|
|
## Stop and report if
|
|
|
|
- The branch check fails; the before block is not found exactly once; step 2 differs (a
|
|
different test fails, or it passes); `cargo check` warns; a test in
|
|
`scheduler_heartbeat` fails; the gate's last line is not `gate: ok`; you edit the
|
|
unreadable-prompt path; you change cron `Finished` / `Failed`.
|
|
|
|
## What the given tests pin
|
|
|
|
A due heartbeat whose inference returns HTTP 400 still pushes `Heartbeat` and is not due
|
|
again 30 s later (`every_s` 60). The existing six heartbeat tests stay green: `every_s` 0,
|
|
busy skip, unreadable prompt, and a successful second tick before `every_s`.
|