Skip to content

feat(agent-task): make the detached local cook a daemon-owned controller job - #11857

Merged
chubes4 merged 4 commits into
mainfrom
feat/cook-job-driver
Aug 7, 2026
Merged

feat(agent-task): make the detached local cook a daemon-owned controller job#11857
chubes4 merged 4 commits into
mainfrom
feat/cook-job-driver

Conversation

@chubes4

@chubes4 chubes4 commented Aug 7, 2026

Copy link
Copy Markdown
Member

Closes the flagship item of epic #11839.

The finding

daemon/controller_job_driver.rs defines a daemon-owned driver with exactly what detached orchestration needs — prepare/execute/resume/cancel, checkpoint, is_cancelled, public_* projections, and HTTP control (POST /controller/jobs/:id/start, /cancel, GET /jobs/:id/events).

Registered drivers, entire tree: CleanupJobDriver, LabStagingDispatchDriver, AgentTaskPromotionJobDriver. Not cook, not fanout, not loop, not controller.

The layer that most needs detachment, resumption, cancellation and inspection ran foreground, while the primitive providing all four was registered for cleanup.


Shape B, and the evidence is not close

I asked for Shape A — daemon executes the cook in-process — if it could be shown safe. It cannot. Three facts compose into a disqualifying argument:

  1. agent_task_secrets.rs:300 — the ambient process environment is the first secret provider, ahead of configured agent-task secrets:
    SecretEnvValueProvider::new("env", |name| env::var(name).ok())
  2. command_runner.rs — the main provider invocation never calls env_clear. The only env_clear in the file is the bounded readiness probe, explicitly commented "Runtime readiness receives no ambient credentials." The real provider inherits the full environment of whatever process runs it.
  3. daemon/control.rs:2364spawn_and_wait_for_lease_attempt builds the daemon Command with neither .env_clear() nor .current_dir(). The daemon is long-lived, shared, and auto-started by LocalControllerJobClient::connect() → ensure_running. Its environment and cwd are a snapshot of whichever caller happened to start it first — possibly days earlier, from a different shell or repo. reattach_exact_live_owner even treats "explicit HOME environment" as part of daemon ownership identity, confirming HOME varies per daemon.

Together: a daemon-hosted cook would silently resolve provider credentials from a different environment than the operator's shell — a different account, or none — with the outcome depending on who started the daemon. That is precisely the silent environment regression that would be worse than a less elegant design.

So: the launcher still spawns the child, preserving env and cwd byte for byte. The daemon owns the durable job, checkpointing, cancellation and HTTP inspection, supervising a child it did not create.

Resume idempotency — met more strongly, by a different route

I did not wire claim_continuation_for_recovery into resume, contrary to my own brief. The continuation machinery re-runs a cook, which inside the daemon means executing provider work in the daemon's environment — the exact hazard above.

Instead the driver never spawns provider work at all, so resume cannot double-run. resume_disposition() is exhaustive over three variants, none of which execute:

  • replay a completed job
  • re-adopt a child still provably alive
  • observe the durable outcome of a dead one

Liveness is PID plus kernel ProcessStartIdentity, so PID reuse cannot alias a stranger.

Consequence to accept: an orphaned cook terminalizes as Failed rather than continuing. Continuation stays an explicit operator --continue, run in a shell with the right environment. That is the honest trade given the environment finding.

Cancel

Delegates to agent_task_lifecycle::cancel_run — the established path, no second mechanism. Pre-materialization it terminates the detached child's tree guarded by an exact start-identity match; post-materialization it follows the Cook alias to the live attempt, which the running cook's own supervisor turns into tick.stop_now → terminate_process_tree(std::process::id()).

Redaction

Exposed: schema, idempotency key, phase, cook_id, run_id, terminal state.
Withheld: child_pid, child_start_identity, launcher log path, and all error textpublic_error carries only the typed code, because cook errors quote provider output which quotes the prompt.

Progress and result are projected field by field, so a future private field cannot escape by default. The prompt, provider invocation, notification route, gate policy and worktree never enter the job at all — they live behind cook_id in the recipe. validate_secret_references is enforced structurally by deny_unknown_fields on both structs: an inline secret is a parse error, not an accepted field.

Scope

Locally-placed agent-task cook --detach-after-handoff only. fanout, loop, controller, Lab-placed cooks and the runner-side path are untouched, and the runner-side refusal (the process is already the runner's owned execution) is preserved with its existing test.

Both mechanisms coexist by design — the detach path is extended, not replaced. Submission is best-effort: an unreachable daemon degrades to today's PID-owned behaviour, reported as controller_job.state: "unavailable". Daemon ownership is attempted, not guaranteed.

The 30s handoff poll survived deliberately. It is no longer load-bearing (submit returns a job id synchronously, and the cook id is durable before the child exists), but deleting it would regress callers reading run_id, which genuinely does not exist until the cook materializes its first attempt. It is now safe to set to 0, which it was not before.

Verification

  • cargo check --workspace --tests -j2 — clean
  • RUSTFLAGS=-Dwarnings cargo build -p homeboy --bin homeboy — clean
  • cargo test -p homeboy-agents --lib cook_job11 passed, 0 failed, including cancelling_a_supervised_job_terminates_the_detached_child and resume_observes_rather_than_restarts_a_dead_child

Nothing outside homeboy-agents / homeboy-cli was touched — homeboy-core needed no changes; the primitive was reused as-is.

Flagging

  • The supervision loop itself is not unit-tested. ControllerJobHandle::new is pub(crate) in core, so a handle cannot be constructed from homeboy-agents. resume_disposition() was extracted so the idempotency decision is tested without one, and the cancel path is covered by a real child-kill test — but the loop is verified by reading. Follow-up: a test-support-gated constructor in core. This is the main residual risk.
  • The fourth commit is a use-after-move the author found and fixed by inspection (start_identity consumed by record_detached_cook_handoff_child); kept separate so the bug and its discovery stay visible.

chubes4 added 4 commits August 7, 2026 11:52
…ooks

A locally-placed detached Cook was PID-owned: no durable job record, no
checkpoint, no cancellation authority, no HTTP inspection. The daemon's
ControllerJobDriver primitive provides all four, but was registered only
for cleanup, lab staging, and promotion.

CookJobDriver makes the daemon own a detached cook's lifecycle while the
launcher-spawned child keeps executing in the operator's environment.
That split is deliberate: the ambient process environment is the FIRST
secret source for a provider invocation, the main provider invocation
never env_clears, and the daemon inherits env and cwd from whichever
caller first started it. Executing a cook inside the daemon would
silently change which credentials the provider receives.

Because the driver never spawns provider work, resume cannot double-run
an attempt; it re-adopts supervision of a child identified by PID and
kernel start identity. cancel delegates to agent_task_lifecycle::cancel_run,
the established path that terminates the detached child's tree before
materialization and drives the in-flight supervisor stop after it.
The launcher now hands durable ownership of a detached cook to the
daemon: submit returns a job id synchronously and start releases the
supervising worker. The child is still spawned here, which is what keeps
the provider's execution environment identical to the operator's shell.

Submission is best-effort. By the time it runs the child is already
spawned and the handoff parent is already durable, so an unreachable
daemon degrades to exactly the previous PID-owned behavior instead of
failing an operator's run. The envelope reports which happened via an
additive controller_job field; every pre-existing field, the exit code,
and the runner-side refusal are unchanged.

The 30s handoff poll is retained at its historical default so no caller
reading run_id regresses, but it is no longer load-bearing: both the
cook id and the job id are addressable the moment the envelope prints,
so the timeout is now safe to set to 0.
…ncel

Adds a testable resume-disposition seam so the idempotency property can
be asserted without constructing a daemon job handle, which core keeps
crate-private.

Covers: the launcher submission round-trips through the driver; the cook
id is the idempotency key so a replayed submit converges on one
supervisor; inline secrets (prompt, env, provider invocation, route, log
path) are refused by deny_unknown_fields; public projections withhold
the child pid, start identity and paths, and project progress/result
field by field; resume replays a completed job and observes rather than
restarts a dead child; an unfinished cook terminalizes as failed; and
driver cancellation actually terminates a live detached child.
…bmission

record_detached_cook_handoff_child consumes the start identity, so the
submission that follows needs its own clone. Also skips submission when
the handoff parent is already terminal: that cook has no lifecycle left
to own, and submitting would start a daemon only to supervise a child
this launcher just killed.

Adds the degradation-contract test: an unreachable daemon must still
return every pre-existing envelope field unchanged.
@chubes4
chubes4 merged commit 9a46cb9 into main Aug 7, 2026
1 check failed
@chubes4
chubes4 deleted the feat/cook-job-driver branch August 7, 2026 12:23
chubes4 added a commit that referenced this pull request Aug 7, 2026
…oller job (#11871)

* feat(agent-task): add a daemon-owned controller job driver for cook batches

The fanout coordinator was an unconditionally blocking thread pool. A
`std::thread::scope` joins before returning, so there was no cancellation
token, no checkpoint, and no resumable driver: when the caller disconnected,
every in-flight child was orphaned and needed a manual `fanout resume`.

Add `CookBatchJobDriver`, the sibling of the Cook driver merged in #11857 and
deliberately the same shape. The daemon owns the durable job; a detached child
process runs the coordinator. That split is not a shortcut — the ambient
process environment is the first secret provider, the real provider invocation
never calls `env_clear`, and the daemon inherits its environment from whoever
started it first, so provider work executed inside the daemon would resolve
credentials from an environment no operator chose. A batch multiplies that
hazard by its width.

The durable request is a batch id and nothing else. `persist_fanout_run_batch_record`
and `persist_batch_cook_recipes` both run before the first child dispatches, and
`resume_cook_batch` already rebuilds an entire wave from a batch id alone, so the
id is a complete durable handle exactly as `cook_id` is for one Cook.

Give the coordinator itself an opt-in control so it can answer to that owner:
a durable cancellation check and a durable-terminality check in the claim loop,
plus per-child terminal publication. Every field defaults to off, so existing
callers keep today's behaviour byte for byte.

Batch cancellation cannot be inferred from child state — an unclaimed child has
no lifecycle record to cancel, and a wave whose early children succeeded
aggregates to `partial_failure` rather than `cancelled` — so record it as its
own explicit fact in the batch record's metadata, where `status` cannot erase it
while recomputing the aggregate.

* feat(agent-task): make --detach-after-handoff real for a locally-placed fanout

The flag is global and advertised on `fanout cook-batch` and `fanout run-plan`,
but every gate that acted on it tested for Cook — `detached_cook_can_queue` and
`is_local_detached_cook` both match `AgentTaskCommand::Cook(_)` — and
`run_split_placement_fanout` returns before any detach handling on local
placement. So `fanout cook-batch --detach-after-handoff --placement local`
accepted a flag promising the caller could disconnect, then blocked that caller
for hours and died with its terminal.

Serve it. `local_detach_fanout` re-executes the coordinator in its own session,
pins the fanout id onto its argv so the daemon and the coordinator agree on
which batch is being supervised, submits the wave as a `CookBatchJobDriver`
controller job, and returns a bounded handoff naming the batch. The launcher
spawns; the daemon supervises. A daemon that cannot be reached degrades to a
PID-owned detached coordinator and reports `controller_job.state: unavailable`.

Where detachment cannot be honoured it now refuses instead of lying. `fanout
plan`, `submit`, `submit-batch`, `status`, `resume`, `artifacts`, a
`cook-batch` without `--run-plan`, and a `--dry-run` wave all return promptly
and own no coordinator to hand over, so each is a hard validation error naming
the command that can be detached instead.

The coordinator learns it has a durable owner from an environment signal the
launcher sets, matching the `HOMEBOY_RUNNER_HOSTED_EXEC` precedent. It carries
the batch id rather than a bare boolean, so a variable inherited from an
unrelated ancestor arms nothing.

Also leaves a documented hook where the wave-completion notification belongs.
A ten-child wave still delivers ten unrelated cook messages and never says
'wave done, 7 green, 2 need attention', even though the batch report has
already computed those totals. The emitter is owned by another change in
flight, so this marks the seam and the payload rather than reaching into it.

* test(agent-task): cover the daemon-owned batch coordinator's correctness properties

Four properties, asserted at the boundary each one actually lives at rather
than through the report, which can be the right shape for the wrong reason.

- A daemon-owned coordinator never starts a child that is already durably
  terminal. Asserted on the dispatch boundary, because the property is about
  work started: re-running a finalized child means a second provider attempt
  and a second pull request.
- An unowned coordinator still runs every child. This is what protects every
  existing caller from the change. Asserted on the cell's provenance rather
  than the dispatch list, so it holds regardless of what `run_cook` itself
  decides about a cancelled cook alias.
- Cancelling a wave stops it starting further children, including children with
  no lifecycle record at all — which is the case a child-state-derived signal
  cannot cover and the durable marker exists for.
- Each child's terminal outcome reaches the batch record as it happens, keyed
  the same way `resume_cook_batch` keys it, so a live coordinator and a resumed
  one converge on one view.

Fixture terminality goes through `cancel_run` rather than a synthesized state,
so the fixture and the mechanism under test cannot drift apart.
chubes4 added a commit that referenced this pull request Aug 7, 2026
An out-of-process orchestrator could already list, inspect, watch and cancel a
detached cook through the generic controller-job surface — cook and fanout are
both `ControllerJobDriver`s (#11857, #11871) — but had no way to read the
durable agent-task *run* the job supervises. A run is not a job.

Resolve it through the activity agent-task provider's `probe_by_id`, which is
already documented as an indexed, non-mutating lookup (#10308) and already
understands Cook-id aliasing.

Deliberately NOT `agent_task_lifecycle::status()`, the reconciling read behind
the CLI:

- it rewrites the durable record on the way out, and a GET that mutates is a
  decision, not something to replicate silently
- for a non-controller-local record it performs a live runner round trip. The
  daemon accept loop handles one connection inline before accepting the next,
  so an unbounded read stalls every other client of a shared process
- `require_run` in this module already refuses the same reconciling facade for
  the same reason (#6768)

The response says `reconciles: false` and names the command that does, rather
than letting a caller assume freshness.

Bounding: exactly one indexed probe, no fallback. `activity::show_activity`
would fall back to a full-corpus scan of up to 1000 records across three
stores; that is unbounded work on a serial loop, and wrong on an agent-task
route anyway. The id is length-capped before it reaches the record store.

Redaction: the `ActivityItem` projection is a typed field-by-field allowlist —
ids, timestamps, state, and evidence *references*. `command`/`cwd` are `None`
for an agent-task record, so no prompt or provider output is reachable. A
failing probe is reported as a flag, never as its message, because error text
from this subsystem can quote record contents and the daemon copies
`message`/`details` straight into the response body.

Auth posture is identical to its neighbours: the read-only API takes no bearer
token and is protected by the daemon's hard loopback-only bind validation.

No submit or retry route: submission has its own safety review.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant