Repository navigation
T65 — Run on_activate again when a box resumes, if its spec sets hooks_on_resume - #3123
minimal-foundry[bot] wants to merge 11 commits into
Conversation
A box whose stored spec sets execution.hooks_on_resume runs its on_activate hooks again when an attach resumes it (BOX-028). A hook that fails stops the host, writes back the record the resume read, and refuses the resume with the hook's source and status.
[session] and [tasks.*] entries carry hooks_on_resume for the box spec (BOX-028). The unknown-field warning no longer names it.
…ivate-again-when-a-box-resumes-if-its
|
Important Review skippedBot user detected. To trigger a single review, invoke the
 |
|
/review |
|
Approved: no findings at Nothing to do. Clobber: none seen in the touched code. |
There was a problem hiding this comment.
arch-review: CONDITIONS · 2 conditions · the full ruling
The rerun of on_activate on resume is in the correct place, and it refuses correctly when a hook fails. It does not fail closed when the composition sidecar is missing. Then has_hooks_for answers false, and the box resumes without its on_activate. Line 499 of architecture.md says a box with hooks_on_resume = true runs on_activate again on resume. C1 makes that path refuse. C2 updates the stale BoxSpec::hooks_on_resume doc comment in crates/mfile/src/spec.rs, because it still points to #3111 as open work.
Outside the diff
crates/mfile/src/spec.rs:77: C2 · This comment says the daemon does not read the key and points to #3111. This PR closes #3111. Say thatminimaldreads the key at resume inBoxActor::resume_ended, and remove the issue link.
Ruled at 9724a10 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
/revise Three things on head 9724a10. The plan now also names crates/mfile/src/spec.rs, an owner-approved amendment for C2.
-
Base merge. Merge
maininto this branch. It conflicts indocs/reference/minimal-dot-toml.mdonly, after T17 — Read the legacy tables for one release with a hint #3105 (T17, legacy tables) merged. Keep both sides: this PR'shooks_on_resumekey, and main's entry-table and legacy-release text. -
Arch C1 (arch_rulings 1044). In
crates/minimald/src/box_actor.rsrerun_activate_hooks():- Before the
has_hooks_for(event)check, matchself.inner. - When it is
BoxInner::Active { composition: None, .. }, logtracing::info!withrefusal = "hooks"and returnErr(..). - The refusal says the session cannot resume because the daemon cannot read its hook settings. As the next step, it names
min session destroy <name>and thenmin session activate. Follow AGENTS.md "User-facing text", and pin the exact text in the test. - Keep the
Ok(())/activate_hooks = 0path only for a composition that is present and declares noon_activate. - Add a test in
crates/minimald/src/box_actor/tests.rs. It removes the composition sidecar of ahooks_on_resumebox, resumes the box, and asserts the refusal and the unchangedstoppedstate.
- Before the
-
Arch C2. In
crates/mfile/src/spec.rs, in the doc comment onBoxSpec::hooks_on_resume(lines 77-79), replace "the daemon does not read this key yet. See T65 — Run on_activate again when a box resumes, if its spec sets hooks_on_resume #3111" with a sentence that saysminimaldreads the key at resume (BoxActor::resume_ended) and runson_activateagain. Remove the PR body's note that calls this an unfiled follow-up.
Change nothing else.
…ivate-again-when-a-box-resumes-if-its # Conflicts: # docs/reference/minimal-dot-toml.md
Without its composition sidecar the daemon cannot see the box's on_activate hooks, so the resume is refused and names the commands that start the session over.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The two new commits close the open points from the ruling on 9724a100. A resume with hooks_on_resume set now refuses when the daemon cannot read the box's composition, so a hidden on_activate cannot be skipped. The BoxSpec::hooks_on_resume doc comment now says that minimald reads the key. The merge of #3105 is clean, and the docs now name [defaults.session]. The change conforms to architecture.md line 499. One note for later work: the mfile test uses only the legacy [session] table, which release 0.9 refuses.
Ruled at 373ffdd against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 1 of 10 on a /revise note, pushed as Resolved conflicts in:
Verify Review threads the coder judged:
A box whose spec sets hooks_on_resume = true runs on_activate again when it resumes. A box without the key runs only on_attach. A hooks_on_resume box whose composition sidecar is missing does not resume. It stays stopped, and the refusal names Notes from the agent: Round 1. (1) Merged main and resolved the conflict in docs/reference/minimal-dot-toml.md. Both sides are kept: main's interactive-task text and entry tables, and this PR's [tasks.*] hooks_on_resume paragraph. That paragraph now links to |
|
/revise Fix the finding on |
A wedged sandbox build stalled the box actor and the resume forever. The launch now uses HOOK_LAUNCH_TIMEOUT, as the destroy launches do. On timeout the resume is refused and the record is written back.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare adds a deadline to the host launch that a hooks_on_resume resume needs. On the deadline, the resume is refused and the box stays stopped or exited. on_activate does not run. Line 499 of architecture.md permits only two results: the resume runs on_activate again, or it does not resume. This change keeps that rule, and the earlier PASS stands for the rest. Update the stale doc comment on HOOK_LAUNCH_TIMEOUT in a later change. It is not a design item.
Ruled at 0208fd9 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
Round 2 of 10 on a /revise note, pushed as
Verify Review threads the coder judged:
A box whose spec sets hooks_on_resume = true runs on_activate again when it resumes, and a box without the key runs only on_attach. When the hook launch on resume does not start within HOOK_LAUNCH_TIMEOUT, the resume is refused, the box stays in the state it resumed from, and the refusal names Notes from the agent: Revise round 2: only the timeout finding is addressed. rerun_activate_hooks now wraps launch_host_for_hooks in HOOK_LAUNCH_TIMEOUT. On timeout it logs a warning with box_id and returns a refusal, and the existing resume_ended error path stops the host, writes back the record it read, and marks the name stopped. A new cfg(test) seam, resume_launch_seam, delays the launch and shortens the timeout for one box id. The new test hooks_on_resume_launch_timeout_refuses_resume_keeps_state pins the full refusal text: "session |
A hooks_on_resume resume launches a host for on_activate before the attach's launch. If that launch failed, finish_resume saw a host and recorded the box as resumed. PendingResume now records that the resume launched the host, and a failed launch stops it and restores the record.
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare conforms to the design. At the earlier head, a failed attach launch after an on_activate rerun recorded resumed for a box that did not run. Now finish_resume stops the host that the resume launched, restores the record and marks the name stopped. Line 499 of architecture.md records resumed only for a box that resumes, so the fix is correct. No test covers the new branch, and minimal-review owns that gap.
Ruled at 74b5425 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
There was a problem hiding this comment.
minimal-review — approval withheld
Verdict COMMENT — APPROVE withheld: 2 unresolved threads — minimal-foundry[bot] crates/minimald/src/box_actor.rs, minimal-foundry[bot] crates/minimald/src/box_actor.rs:1555. No findings.
Panel: 3 of 3 personas ran (correctness, security, supply-chain).
|
Round 3 of 10 on a failed check suite, pushed as
Verify Review threads the coder judged:
A box whose spec sets hooks_on_resume = true runs on_activate again when an attach resumes it. A box without the key runs only on_attach. If the attach launch fails after on_activate ran, the box stops and goes back to the state it resumed from. It does not stay running on the hook host. Notes from the agent: Round 3. The test-kvm failure excerpt shows only the step scripts and the post-job cleanup. It does not show a failing test or step that this diff touches, so I could not find a cause in this change. The minimald, mfile and resume tests pass on this box (cargo nextest run -p minimald resume: 31 passed). If the KVM lane fails again, a person must read its full log. The new rollback path has no test. To make attach_launch fail after the hook host exists, a test seam in crates/minimald/src/box_actor/attach.rs is necessary, and that file is outside the task's layers. |
There was a problem hiding this comment.
minimal-review — approved
Verdict APPROVE — every review thread is resolved; @agent-137 closed the last one. The review of 74b54256d7be withheld this approval on the threads open then. Posted as a comment: the App cannot approve its own pull request; the check and the label carry it.
…ivate-again-when-a-box-resumes-if-its # Conflicts: # crates/mfile/src/lib.rs
There was a problem hiding this comment.
arch-review: PASS · the full ruling
The compare is only a merge of main. The daemon change for BOX-028 is the same as on 74b54256, and that head had a PASS. The merge rebuilds BoxSpec::build so that hooks_on_resume and the new timeout both come from the entry's table. Both serialize under execution, as architecture.md line 1099 says. The change still conforms to line 499. No conditions.
Ruled at a6087e6 against gominimal/arch@7a86ec7bb3c0 by claude-opus-5-5.
|
A merge of main before round 4, not counted against the cap (1 of 3), pushed as Resolved conflicts in:
Verify Converted to a draft: something above needs a person before review. A box whose spec sets hooks_on_resume = true runs on_activate again when it resumes. A box without the key runs only on_attach. The branch now merges cleanly with main. Notes from the agent: This round resolved the merge conflict in crates/mfile/src/lib.rs. Main changed the [tasks.*] unknown-key check to use unknown_entry_keys, which drops the timeout key. The resolution keeps that helper and also drops hooks_on_resume, as the [session] check does. The other merged paths came from main without conflicts. cargo-nextest is not installed on this box, so each test line ran with cargo test and the same package and test name. cargo clippy -p mfile --all-targets printed no warnings. |
T65 — Run on_activate again when a box resumes, if its spec sets hooks_on_resume
slice: S4
kind: task
covers: BOX-028
size: M
est: 120 code, 210 tests
executor: agent
difficulty: needs care
model: standard
diagnostics: daemon log names each on_activate run on resume and its outcome; a failed hook refuses the resume naming the hook's source and status
observability: box.resume span records hooks_on_resume and the activation hooks run
layers:
- daemon:
- crates/minimald/src/box_actor.rs: resume_ended runs on_activate when the stored spec sets execution.hooks_on_resume; a failed hook restores the record and refuses
- crates/minimald/src/box_actor/tests.rs: tests
test: cargo nextest run -p minimald hooks_on_resume_reruns_on_activate
test: cargo nextest run -p minimald hooks_on_resume_failing_on_activate_refuses_resume_keeps_state
test: cargo nextest run -p minimald resume_runs_only_on_attach_hook
- config:
- crates/mfile/src/lib.rs: hooks_on_resume is not an unknown key
test: cargo nextest run -p mfile hooks_on_resume_entry_key_is_not_reported_unknown
- docs:
- docs/reference/minimal-dot-toml.md: hooks_on_resume key under [session] and [tasks.*]
produces: none
consumes:
- crates/mfile/src/spec.rs
proof: A box whose spec sets hooks_on_resume = true runs on_activate again when it resumes; a box without the key runs only on_attach.
depends on:
- blocked by #2415
Proof
A box whose stored spec sets execution.hooks_on_resume = true runs its on_activate hooks again when an attach resumes it, before on_attach. If one of those hooks fails, the resume is refused with the hook's source and exit status, and the box stays stopped or exited. A box without the key still runs only on_attach on resume.
Notes from the agent: resume_runs_only_on_ 8000 attach_hook existed already and passes unchanged. On a failed on_activate the resume stops the host it launched for the hooks, writes back the record it read, marks the name stopped, and refuses with channel exit status 1. Status 1 is new: 2 stays the terminal and name refusals, and 3 stays the stored-spec refusal. The refusal reads: "session ." with the captured hook output after it. The box.resume span now records hooks_on_resume and activate_hooks (the count that ran). Each rerun is logged with its source and status. Follow-up outside layers: the doc comment on BoxSpec::hooks_on_resume in crates/mfile/src/spec.rs (a consumes file) still says the daemon does not read the key yet. It needs a one-line update in a later change. just lint-prose did not run, because vale is not installed on this box. The whole-crate suites and just ci did not run.
<name>cannot resume, and it stays . Its activation hook fromTests
cargo nextest run -p minimald hooks_on_resume_reruns_on_activatecargo nextest run -p minimald hooks_on_resume_failing_on_activate_refuses_resume_keeps_statecargo nextest run -p minimald resume_runs_only_on_attach_hookcargo nextest run -p mfile hooks_on_resume_entry_key_is_not_reported_unknownVerify
min task run agent-verifytimed out after 1800s, so this PR is a draft.Base
The base branch moved while the coder worked; it was merged in before the tests ran.
Closes #3111