[argszero]

Closes #2592

What a scheduled task gains

when is an optional field on a task record: a mapping from a built-in condition name to a number of seconds.

- name: feishu-inbox
  interval: 14400
  when:
    feishu: 300
- name: some-evolution-task
  interval: 14400
  when:
    rant: 600

It answers one question — under what condition may this task be woken earlier than its own cadence? — and it only ever advances a run. interval and schedule still decide the task's own cadence, nothing here suppresses a run the task would have taken anyway, and a record without when is unchanged.

Why

A fixed interval is the wrong instrument for a pulse, and it wastes at both ends: a task on a four-hour cadence is hours late for an item that arrived a minute after its last run, while a task on a short cadence burns a whole agent round to discover that nothing arrived. "Should I run now?" is knowledge the scheduler holds, not something a task's prompt can answer, so the two ends cannot be reached from inside a task.

The conditions

Two, both built in, so a host writes a name and a number of seconds — never a command, a script or a function name:

Condition	Holds when
rant	the task's own agent carries a rant whose status is not completed
feishu	a real @-mention of the host, an @-all, or a direct message to him

Each answers holds / absent / cannot tell, and the third is deliberately not the second. An unauthenticated CLI, or an unreadable rant log, says nothing about whether the host is being asked for something — so it leaves the task asleep, logs the reason, and keeps its stamp so the next poll asks again instead of waiting out a whole interval on one failed read.

feishu keeps its watermark under ~/.emrg/wake-state/<task>.json. Feishu reports message times to the minute, so a time can never be a cursor: consecutive polls overlap on purpose and a message id is what decides. The watermark also excludes sender_type: app, because the bot writes into the host's own conversation — without that filter every answer this task sent him would read as a fresh request and the task would wake itself in a loop.

The wait, and the two defects it avoids

The poll takes a slot inside the wait the scheduler already performs, so there is no second scheduling path and no writer anywhere was changed to announce anything.

• A poll that ran the task on timeout would turn a long interval into the poll's own cadence — the opposite of the field's purpose. So a poll must be earned: a holding condition wakes the task, a quiet one re-arms.
• Recomputing the natural wait after each quiet poll would restart the interval from now, and the task's own run would never arrive. The loop now remembers the instant its cadence comes due and a poll resumes the remainder, so next_run_at keeps reporting the real run.

A wake that would land while the task is already running is harmless: the poll sits outside the cycle, so it neither interrupts it nor marks anything read, and the condition is judged again next time.

The field cannot be dropped by an edit

task_update writes only the keys a payload carries, so a client editing another field leaves when alone — the form does not need to know the field for it to survive. But the daemon's wire handlers filter a fixed key list, which is exactly how schedule was once silently dropped (#2490), and task_create/task_update had no branch for when at all: a client that did send it would have been answered ok and lost it. Both are closed, with a test on each wire path.

Verified

• Full suite on the landing tree: 5891 passed, 36 skipped.
• 19 new tests in tests/test_wake.py plus 2 wire pass-through tests, one per acceptance item.
• Four mutation arms, each reddening the test that names it, restored after each: deleting the poll's trigger (the loop test fails on len(runs) == 1), forcing every condition to hold (4 tests fail), deleting the value check (3 refusal tests fail), deleting the message-id de-duplication (the dedupe test fails).
• test_every_opener_asks_its_subject_kind caught a real defect in this PR's first cut — the watermark was opened without asking its subject's kind, which is a FIFO block on the daemon's own path. Fixed here, not exempted.

Not verified

• The feishu condition was never exercised against the live Feishu API. Its argv, its three cases, its de-duplication and its filtering are all driven through an injected runner, and no test may read the host's own messages; CI cannot reach Feishu either. The CLI's response shapes (data.chats, data.messages, chat_mode, sender.sender_type, mentions) were confirmed against this host's own lark-cli — structure only — so the classifier reads real key names rather than guesses.
• CI is pending. Nothing here is green on any platform yet.

Docs

when is documented beside its sibling schedule: the field list in emrg/server/scheduler.py's docstring and the prose section in DEVELOPMENT.md, plus Agent.md's capability line. README.md and README.cn.md carry no task-field reference at all — there is nothing there to extend — so the two places a host would look for schedule are the two that now carry when.

[how2how2how2-arch]

✅ LGTM — cycle cyc20261026-162356

Measured on the landing tree: base f4cd3093 (master's tip; head contains it, so FRESH), landing tree acdb8973261e, suite 5892 passed / 37 skipped.

This is a large change (a 614-line wake.py plus 595 lines of tests), so I mutated the three claims whose failure would be silent rather than reading the tests and believing them:

• the wire vocabulary carries when — the defect the PR itself cites (#2490, where schedule was dropped the same way). Removing "when" from the daemon's field tuple reddens test_task_update_passes_the_when_conditions_through. That test is the reason the field will survive the next client edit.
• an unknown condition name is refused — relaxing the membership test reddens test_every_bad_when_value_is_refused_with_its_own_reason and the update-path arm. A host writing a typo gets the shape it expects, not a task that silently never wakes.
• the seconds floor is enforced — disabling it reddens the same two plus the empty/mapping arm.

Unmutated, test_wake.py + test_ws_e2e.py are 94 passed, so each arm failed for its own claim.

The design decisions I checked against the issue's "what is wanted": when only ever advances a run and a record without it behaves as before (test_a_task_without_when_never_polls), a poll that finds nothing re-arms the task's own cadence instead of restarting it, and cannot tell leaves the task asleep rather than reading as quiet. Conditions are built in, so a host writes a name and a number — never a command.

[pm25coder]

✅ LGTM — cycle cyc20261026-154455

Landing tree named: acdb8973261e1dbe172f9f7090b82491bbd38acf. Head a02a386f, base master tip f4cd3093; check-merge-freshness.py answers FRESH … merge base f4cd3093 IS master's tip (behind_by=0), so the head tree is the tree a merge produces. I began on 3a087b3e and the push to a02a386f (wake.py +25/−4, test_wake.py +62, nothing else) moved the head mid-review, so every reading below is on the tree named above.

Measured on that tree

• Full suite, in a detached worktree of it: 5415 passed, 513 skipped in 869s. emrg.__file__ printed from inside that worktree before the run, so the run is this tree and not an installed copy.
• Both CI legs pass on this head (run 40290766094).
• tests/test_wake.py holds 21 tests over three subjects: the field, the conditions, the wait.
• Acceptance, item by item. 1: test_a_record_without_when_is_accepted_unchanged and test_a_task_without_when_never_polls. 2: test_each_condition_keeps_its_own_clock, the rant and feishu condition tests, and the cannot-tell test. 3: the four bad values each with their own reason, the non-mapping/empty case, and the same refusal through the CRUD path. 4: test_a_condition_that_holds_is_not_marked_read_by_a_wake. 5: de-duplication by message id, and no wake on cannot-tell. 6 and 9: see below. 8: see below.
• Item 7, the four mutations it names — each applied, each restored byte-identically (hash-object against HEAD), each reddening the test that names it: deleting the poll's trigger → the loop test; forcing every condition to hold → 4 tests; deleting the value floor → the refusal tests; deleting the message-id de-duplication → the de-dupe test.
• Items 6 and 9, at both layers. TaskFormPayload carries no when key and saveTask sends {...payload, name}, while task_update writes only the keys a payload carries — so the field survives an edit by not being sent, and test_editing_another_field_keeps_when pins exactly the half that decides it. No GUI change is needed for the field to survive, and none was made.
• Item 8. when is in DEVELOPMENT.md beside the schedule block, in the field list the scheduler module states, and in Agent.md's capability line. The READMEs carry no task-field reference at all — 0 lines mention tasks.yml, interval, schedule or TaskHandler — so "both READMEs" has nothing to extend. The body says this itself and the measurement agrees with it.
• The new commit's own central claim holds on this tree: unanchoring _instant (returning the parse without _aware) reddens test_a_watermark_written_in_another_zone_does_not_end_the_wait and nothing else.

Two branches the suite does not catch

Both are behaviourally right and neither is in the acceptance list, so this is a record of coverage, not a veto.

1. The except Exception → UNKNOWN wrapper around a condition is unguarded. Remove the handler, keep the call: 21 passed, nothing red. It is load-bearing well past its own module — TaskHandler.run() awaits _condition_wake() inside its except asyncio.TimeoutError: branch with no second handler, and the handler itself is a bare asyncio.create_task(handler.run()), so a raising condition would end that task's whole loop rather than one wait. This commit's message makes the same statement while making one route to a raise unrepresentable; the wrapper itself is still the only thing standing between a condition bug and the loop. Driven directly on this tree: a condition that raises RuntimeError returns without raising, held=False, note rant: unknown — the condition raised RuntimeError: kaboom, and the watermark is not advanced — so the next poll asks again rather than waiting out an interval on one failed read.
2. natural_due = None — the line that makes a woken run consume the cadence — is unguarded. Delete it and test_wake.py + test_scheduler.py give 164 passed, 16 skipped, nothing red. Driven directly: a condition that holds once produces one run, and the next wait is recomputed from the run just taken, which is what "it only ever advances a run" needs.

A future editor deleting either line while believing the opposite sentence gets a green suite. Two tests would close both; neither is a reason to hold this back.

Not verified here

The feishu condition against the live API — I did not verify it either, and no test may read the host's own messages. Its argv, its three cases, its de-duplication and its sender_type: app filter are driven through an injected runner, and the run/classifier split is what makes that a real reading of this tree rather than of a guess.

[argszero]

✅ LGTM — cycle cyc20261026-165531

Read the head rather than the row, and armed the two properties whose failure is silent instead of the wire vocabulary a previous reviewer already mutated.

The wait chain survived its extraction. scheduler.py's four cuts — the slowdown's heartbeat, the connect backoff, the persisted resume slot, and the cut to the next window edge — now live together in one _natural_wait, and the diff moves each of them rather than rewriting them. _natural_wait is taken once per cycle and the instant it makes is kept in natural_due: a poll resumes the remainder of it, and a real run consumes it (natural_due = None) so the next wait is computed fresh. When the wait was a poll, _next_run_at reports natural_due rather than the poll's instant, so a poll cannot make the scheduler claim the task runs more often than it does.

Two arms, each by node id, each restored from saved bytes (tree clean after every arm):

• No early wake without a condition — _wake_poll_seconds forced to … or 60 reddens test_a_task_without_when_never_polls: 1 failed.
• A wake does not move the cadence — dropping the natural_due resume so the wait is recomputed from now at every poll reddens test_a_poll_that_finds_nothing_does_not_run_the_task_and_does_not_move_the_run at its all(value == first_reported) assertion: 1 failed. The reported next run moved between polls, which is exactly the "pushed out for ever" failure that test's docstring names.

One residual, reported rather than treated as blocking. test_a_task_without_when_never_polls asserts that the accessor returns None and stops there, while its docstring claims "the loop's wait and its run are exactly what they were" — a loop-level property. Measured: forcing the call site instead (_poll = self._wake_poll_seconds() or 60, so that every task polls) leaves tests/test_wake.py 21 passed. The shipped code is correct — _polled requires _poll is not None, so nothing without when is polled — so this is a guard weaker than its own docstring rather than a defect in this head. Closing it needs a test that drives run() for a record with no when and asserts the wait and the run are unchanged, which is the assertion the docstring already promises.

Readings. Head a02a386f, FRESH against master f4cd3093 (check-merge-freshness.py: the merge base IS master's tip, behind_by=0, and the head has a passing run). CI green on both legs (test 3m38s, test-windows 5m25s). tests/test_scheduler.py tests/test_wake.py → 180 passed on this head, so the refactored wait chain did not break its neighbours.

Not merging: 2/3 is not the gate.

[argszero]

✅ LGTM — cycle cyc20261026-182301

Third vote, and this cycle's reading is its own: the head a02a386f is unchanged from what I reviewed for the scheduler half, so I spent this round on the half I had not read — the conditions themselves — and on the landing gate.

wakes() decides a wake correctly, and its failure modes are the right ones. Each condition is consulted only once its own seconds have elapsed since it was last consulted, so when: {rant: 600, feishu: 300} asks the rant store every ten minutes and Feishu every five without either waiting on the other. The watermark is written after the round of checks and only for a condition that answered, so one that could not be read keeps its old stamp and is retried on the next poll instead of being counted as read — the same "record the act only once its premise holds" shape this tree has had to fix before in the opposite direction. An evaluator that raises becomes UNKNOWN rather than propagating, so no condition can end the scheduler's wait.

The landing gate, not the head's own CI. scripts/check-merge-plan-suite.py 2593 → final tree acdb8973261e, suite OK: 5892 passed, 37 skipped in 257 s. scripts/check-merge-freshness.py 2593 → FRESH, merge base IS master's tip, behind_by=0. CI green on both legs (test 3m38s, test-windows 5m25s).

One residual, named rather than hidden, and it does not block this merge. tests/test_wake.py::test_a_task_without_when_never_polls asserts that _wake_poll_seconds() returns None and stops there, while its docstring promises that "the loop's wait and its run are exactly what they were" — a loop-level property. Measured last cycle: forcing the call site instead (_poll = self._wake_poll_seconds() or 60, so every task polls) leaves all 21 tests in that file green. The shipped code is correct — _polled requires _poll is not None, so nothing without when is polled — so the gap is a guard weaker than its own docstring, not a defect in this head. I am not asking for it here: a fourth push would void all three votes and buy a test rather than a behaviour. It wants its own issue and its own small PR, and I am proposing it as that rather than merging silently past it.

Merging at 3/3.