feat(goals): add pause/resume and a configurable loop cadence by DaBlitzStein · Pull Request #7973 · librefang/librefang

DaBlitzStein · 2026-08-29T13:30:01Z

Summary

Add GoalRunPhase::Paused (between Running and Finished) and a tick_interval_secs: Option<u64> field on Goal (default 2s, clamped 1-86400 seconds, matching cron's own MAX_EVERY_SECS ceiling).
GoalRunner::pause/GoalRunner::start (kernel): pause is cooperative -- the loop finishes its current turn, checkpoints iteration count and progress into the shared KV (the goal_runs table's phase column has a CHECK constraint that does not admit paused), and exits. start() auto-detects and resumes from that checkpoint when one exists.
stop() (cancel) now also discards any pause checkpoint, so a cancelled goal's next start is a genuine fresh run.
GoalRunner::state() falls back to a persisted pause checkpoint when the in-memory registry has no live entry, so a paused run stays observable via GET /api/goals/{id}/run after its loop task self-cleans.
The run loop now reads tick_interval_secs from the freshly-loaded goal each iteration instead of a hard-wired 2-second constant.
Kernel: LibreFangKernel::goal_run_pause / goal_run_resume (the latter is the same path as goal_run_start -- resume is start-with-a-checkpoint). KernelApi trait gets matching pause_goal_run / resume_goal_run methods.
Fix: goal-run ticks now carry the goal id as SenderContext.chat_id, so send_message_full's session derivation scopes on (agent, "autonomous", goal_id) instead of just (agent, "autonomous"). Previously every loop-mode goal of one agent collapsed onto a single session, so two concurrent runs interleaved their prompts into one conversation history.
API: POST /api/goals/{id}/pause and POST /api/goals/{id}/resume (409 when there is no paused run to resume -- without that guard, resume on a goal with no checkpoint would silently restart it from iteration 0). start_goal_run and the new resume_goal_run share a start_or_resume body.
API: POST /api/goals and PUT /api/goals/{id} validate and persist tick_interval_secs; a blank string or null clears the override back to the default, matching the existing parent_id / agent_id clear-signal convention (Create a Goal, returns 404 #6562).

Out of scope

Two fixes originally scoped for this change -- preserving GOAL_LEARNED: marker case and letting an evaluator complete a goal even without an explicit GOAL_DONE marker -- do not apply to this codebase's current goal runner: there is no learnings-capture mechanism, no evaluator consultation, and no GOAL_LEARNED marker parsing here at all (parse_tick only recognizes GOAL_PROGRESS: / GOAL_DONE / GOAL_COMPLETE / GOAL_BLOCKED). Confirmed via grep -r 'GOAL_LEARNED\|evaluate_goal\|verify_agent_id' crates/ returning no hits anywhere in the workspace. Nothing to fix.

Test plan

Refs #5744

houko

Good change overall — separating "pause keeps the checkpoint" from "stop discards it" is the right distinction, /resume being /start plus a precondition is the right factoring, and rejecting an out-of-range tick_interval_secs at the API while the runner still clamps defensively is the correct division (the operator hears about their number; a bad stored value can never wedge a loop).

One thing to fix: /resume answers 409 for a goal that does not exist.

In start_or_resume, the require_paused guard runs before the goal is looked up:

if require_paused {
    let paused = state.kernel.goal_run_state(goal_id)
        .is_some_and(|run| run.phase == GoalRunPhase::Paused);
    if !paused {
        return (StatusCode::CONFLICT, Json(json!({
            "error": "This goal has no paused run to resume. Use POST /api/goals/{id}/start to begin a new run."
        })));
    }
}

// ... only after this does the handler read the store and 404 on a missing goal

goal_run_state returns None for a goal that was never created, which is indistinguishable here from a goal that exists with no paused run.
So POST /api/goals/<typo'd-uuid>/resume returns 409 with a message telling the operator to POST /api/goals/{id}/start — advice that will fail too, because there is no such goal.
404 is the honest answer, and the well-formed-uuid case is exactly when a typo produces this.

Moving the guard below the store read and the goal lookup fixes it: by then a missing goal has already produced its 404, and 409 means only what it says.
Worth an integration test alongside goal_run_pause_signals_a_live_run — resume against a random uuid, assert 404.

Smaller note, no change needed: validate_tick_interval runs twice on the update path — once before the transaction for the range check, then the value is re-read from req inside it.
That is the right shape (validate outside, apply inside), but the second read repeats the is_clear_signal branch, so if the clear semantics ever change they have to change in both places.
A comment at the in-transaction site pointing at the validator would be enough.

houko

Careful work: rejecting an out-of-range tick_interval_secs at the API while the runner clamps defensively is the right division, the constants carry their reasoning (MIN_… = 1, not 0: a zero-second cadence removes the only gap between consecutive provider calls), and the integration tests cover the null-clears-the-override case that usually gets missed. Two gaps.

Blocking

1. Neither new route has #[utoipa::path], and both drift guards are blind to that by construction.

grep -c "utoipa::path" over this diff returns 0, so POST /api/goals/{id}/pause and POST /api/goals/{id}/resume never reach openapi.json or any generated SDK. CI is green because of how the two guards are scoped — openapi_path_coverage_test.rs's own module doc says it:

The dead-route audit asserts that every path in the spec is wired into the axum router. This test asserts that every handler annotated with #[utoipa::path(...)] in the crate source is actually referenced from ApiDoc::paths(...).

Spec→router and annotation→spec. A handler with no annotation at all is in neither set, so nothing fails. Add #[utoipa::path] to both handlers, list them in ApiDoc::paths(...) in openapi.rs, regenerate openapi.json, and recompute xtask/baselines/openapi.sha256.

This matters immediately: #8029 adds pauseGoalRun / resumeGoalRun to the dashboard client against these exact paths, and the four SDKs are generated from the spec.

2. No changelog fragment.

Pause/resume on autonomous goal runs and a configurable loop cadence are both operator-visible, and the cadence one has a cost story worth telling in the release notes (the MIN_GOAL_TICK_INTERVAL_SECS rationale is already written — reuse it): changelog.d/added/7973-....md.

Coordination

This PR is what makes GoalRunPhase::Paused real. Two open PRs are already written against it:

#8067 widens the dashboard's phase union to include "paused" and adds a Pause-icon badge. Without this PR that value cannot occur; with it, it can.
#8029 adds the pauseGoalRun / resumeGoalRun client functions and mutation hooks for the two routes added here.

Both should land after this one, and #8067's badge work would benefit from #1 being fixed first so its TS types can come from the spec rather than being hand-written.

Non-blocking

pause_goal_run answers 200 {"ok": true, "paused": false} when there was nothing to pause. That is defensible (the request succeeded; the run was already not running), but a client cannot distinguish it from a successful pause without reading the body. Worth one line in the handler doc saying so, since #8029's mutation only checks the HTTP status.
The pause checkpoint lives in shared-memory KV and clear_pause_checkpoint's doc calls it "load-bearing: a checkpoint that outlives its pause would silently seed the next run". Worth an integration test for the daemon-restart case — pause, drop and rebuild the runner, and assert state() still reports Paused from the checkpoint rather than the run having vanished. The fallback path in state() is written for exactly that and is currently only exercised in-process.

houko

Downgrading my earlier review from "request changes" to a comment.

On reflection the two things I raised are additive rather than defects: the code is sound, CI is green, and the design — rejecting an out-of-range tick_interval_secs at the API while the runner clamps defensively — is right. Blocking a green PR on a missing annotation was a heavier call than it deserved, so this is your call to make rather than mine to gate.

Both asks stand, and I would still take them before merge rather than after:

#[utoipa::path] on the two new handlers. grep -c "utoipa::path" over the diff is 0, so POST /api/goals/{id}/pause and /resume never reach openapi.json or the four generated SDKs. CI is green because of how the guards are scoped — openapi_path_coverage_test.rs's module doc says it checks annotated handler → ApiDoc::paths(...), and dead_route_audit_test.rs checks spec → router. A handler with no annotation is in neither set, so nothing fails. It matters immediately because #8029 adds pauseGoalRun / resumeGoalRun against these exact paths.

A changelog fragment. changelog.d/added/7973-....md. The MIN_GOAL_TICK_INTERVAL_SECS rationale already in the source ("a zero-second cadence removes the only gap between consecutive provider calls, turning a run into a tight loop billing tokens as fast as the provider answers") is the sentence the release notes want.

Worth restating the sequencing, since three PRs depend on this one: it is what makes GoalRunPhase::Paused real, so #8067 (dashboard phase badges, widens the union to include "paused"), #8029 (pause/resume controls) and #7784 (TUI tui-goals-phase-paus) all need it first.

Superseded by my follow-up comment — downgraded to non-blocking; the utoipa and changelog asks stand as recommendations.

…-CN and ko The i18n coverage guard fails because the Goals screen references four keys that the Ukrainian locale never defined: tui-goals-phase-mxit, tui-goals-phase-paus, tui-goals-phase-rlim and tui-goals-run-paused. Chinese and Korean carry the same gap, and the guard checks zh-CN too, so a Ukrainian-only fix would have moved the failure to the Chinese assertion. Ukrainian badges follow the file's truncation style (ПАУЗ from ПАУЗА, ЛІМІТ for the rate limiter, МАКС from МАКСИМУМ) and the paused status reads призупинено, matching the виконується/завершено/зупинено series. Chinese badges read 暂停, 限流 and 迭代上限 with the paused status 已暂停, reusing the terms the locale already uses for its run status strings. Korean keeps the English phase badges its five existing siblings use and translates the paused status as 일시 중지됨. The paused phase and status only render once librefang#7973 lands, since GoalRunPhase::Paused and the pause/resume endpoints are not on main yet.

houko

The pause/resume design is right and the reasoning in the doc comments is unusually good — the "why not the goal_runs table" note on goal_pause_key, the ordering constraint on reading the checkpoint before stop_locked clears it, and the goal_tick_sender_context note on why per-goal scoping does not cost prompt-cache reuse are all load-bearing and all correct. The session-scope fix is a real bug fix and its three tests assert the right properties.

Two defects and one convention item before this can go in.

1. `POST /api/goals/{id}/resume` replaces the run's iteration cap with the default, and can terminate the resumed run immediately

resume_goal_run dispatches with no body:

crates/librefang-api/src/routes/goals.rs:206-210

pub async fn resume_goal_run(
    State(state): State<Arc<AppState>>,
    Path(id): Path<String>,
) -> (StatusCode, Json<serde_json::Value>) {
    start_or_resume(state, id, None, true).await
}

start_or_resume reads the cap only from that body, so max_iterations is None on every resume:

let max_iterations = match body.as_ref().and_then(|body| body.0.get("max_iterations")) {
    None => None,
    ...

goal_run_start then substitutes the compiled default (goal_lifecycle.rs:33):

let max = max_iterations.unwrap_or(DEFAULT_GOAL_MAX_ITERATIONS).max(1);

and GoalRunner::start seeds iteration from the checkpoint while taking max_iterations from the argument (goal_runner.rs:635-636):

iteration: resume.as_ref().map(|r| r.iteration).unwrap_or(0),
max_iterations,

ResumePoint.max_iterations is parsed out of the checkpoint at goal_runner.rs:331 and used by state() at :498, but never by start(). That asymmetry is the bug: GET /run reports the paused run's real cap, and /resume then runs it with a different one.

Failure scenario:

POST /api/goals/{id}/start with {"max_iterations": 100}.
The run reaches iteration 30. POST /api/goals/{id}/pause checkpoints {iteration: 30, max_iterations: 100}.
POST /api/goals/{id}/resume. The cap becomes 25.
run_loop's top-of-loop guard (goal_runner.rs:917) sees 30 >= 25 and breaks with MaxIterationsReached before the first turn.
start → stop_locked already cleared the checkpoint, and the non-Paused exit calls clear_pause_checkpoint again, so the run is settled and the 30 iterations of progress are unrecoverable. Resume destroyed the thing it was asked to continue.

It also misfires in the other direction: start with {"max_iterations": 5}, pause at iteration 3, resume, and the run now gets a cap of 25 the operator never set.

The checkpoint value is already loaded — start should prefer resume.max_iterations over the argument when resuming (or resume_goal_run should read it and pass it through). Whichever layer you pick, please also decide explicitly what an explicit max_iterations on a resume body should mean, and say so in the doc comment.

Neither existing test catches this because both resume paths hard-code the cap: pause_checkpoints_and_start_resumes_from_it (goal_runner.rs:1998) calls runner.start(goal_id, agent_id, 100, ...) with the same 100 it started with, and the four HTTP tests cover pause-signalling, the idle-pause false, the no-checkpoint 409 and malformed ids — none drives a successful resume through the route. A test that starts with a non-default cap, pauses, resumes via POST /resume, and asserts the resumed max_iterations is what the operator set is the discriminator.

2. A paused run's `started_at` / `updated_at` are minted fresh on every poll; the persisted `paused_at` is never read

persist_pause_checkpoint stores it (goal_runner.rs:310):

"paused_at": Utc::now().to_rfc3339(),

ResumePoint (:293-298) has no paused_at field and load_pause_checkpoint (:319) does not parse it, so the value is write-only. state()'s checkpoint fallback then stamps both timestamps with the clock (:492-503):

let now = Utc::now();
Some(GoalRunState {
    ...
    started_at: now,
    updated_at: now,
})

So two consecutive GET /api/goals/{id}/run calls on a goal that is not moving return two different started_at values, and any client computing "paused for how long" gets approximately zero, always. For a feature whose point is that a suspended long-horizon run stays observable, the timestamps are the part an operator reads.

The fix is small since the data is already persisted: carry paused_at on ResumePoint, parse it, and use it for updated_at. Please also checkpoint the run's real started_at and restore it, in both state() and start() — start() currently stamps started_at: now on resume too (:639), so a resumed long-horizon run loses the timestamp of when it actually began. If you decide a resume legitimately begins a new segment and should re-stamp, that is defensible, but it needs to be the documented choice rather than a side effect.

3. No changelog fragment

The diff touches seven files and none is under changelog.d/. This adds two HTTP routes, a new GoalRunPhase variant, and a new persisted Goal field — user-visible surface that belongs in the release notes. Per changelog.d/README.md this wants a fragment in changelog.d/added/ named for the PR, ending (#7973) (@DaBlitzStein). Worth two entries if you want the cadence control described separately from pause/resume, since they are independently useful.

4. Nit: `goal_tick_sender_context`'s `display_name` parameter

goal_lifecycle.rs:136-146 takes display_name: &str, and the sole production caller (:53) passes SYSTEM_CHANNEL_AUTONOMOUS — the same constant the function already hardcodes into channel. The three tests pass it too. It is a degree of freedom nothing varies and nothing could sensibly vary, so it reads as configurable when it is not; drop the parameter and use the constant directly in the body.

Verified separately

The "Out of scope" note checks out: GOAL_LEARNED, evaluate_goal and verify_agent_id have no hits in the workspace, and parse_tick recognises only the four markers you list. Nothing to fix there.

* feat(cli): add the goal command and a Goals tab to the TUI Goals had an HTTP surface and a dashboard but no terminal surface at all. This adds both CLI and TUI access to the `/api/goals` endpoints that already ship in the daemon. `librefang goal "<description>" --agent <name-or-uuid>` creates a goal and starts its run. `--max-iterations` caps the run and `--watch` polls `GET /api/goals/{id}/run` every two seconds, printing phase, iteration, and progress until it ends. With `--watch` the exit status reports the outcome: zero when the run finished, one when it stopped, was rate-limited, or hit the cap, so a shell `&&` chain cannot mistake an abandoned goal for a completed one. The agent is resolved through `resolve_agent_id` and validated before the goal is created. `POST /api/goals/{id}/start` refuses a goal with no agent assigned, so creating first and validating afterwards left an unrunnable goal behind on every mistyped invocation. The TUI tab lists goals with a status badge and progress, filters with `/`, creates through a three-field wizard, and starts, stops, or deletes the selection. It takes the `F8` / `Alt+8` slot the retired Channels tab freed rather than renumbering the existing bindings. Run state is fetched per goal from `GET /api/goals/{id}/run` when the detail pane opens. The goal document returned by the list endpoint has never carried `run_phase`, `run_iteration`, or `run_max_iterations` — those live in the kernel's run registry — so reading them from the list payload would leave the phase permanently absent and the start/stop toggle permanently stuck on "start". The toggle keys off that live phase rather than the document's `status`, which is set to `in_progress` when a run starts and never cleared when the loop ends. The list is read from the `items` array of the endpoint's `PaginatedResponse`, with the bare-array fallback the crate already uses elsewhere. All user-facing strings go through Fluent in the four CLI locales (en, ko, uk, zh-CN): 63 keys each, covering the screen, the command, and the `tui-tab-goals` label. Labels carry their own colon inside the translated value so a locale can punctuate it its own way — zh-CN uses the full-width form — and the variable-shaped detail sits in the info hints rather than the labels. Unit tests cover list filtering and wrap-around navigation, the delete confirmation, the create wizard including its refusal to submit without an agent, search mode, and the phase translation table against every `GoalRunPhase` variant the kernel can emit. * docs(changelog): point the goals fragment at its real PR number * feat(tui): add colored phase badges for all goal run states Extends the goal detail pane and list view with distinct colors for paused (yellow), max_iterations_reached (yellow), rate_limited (red), and stopped (gray) phases. Adds the missing "paused" entry to the phase_message_key lookup and corresponding i18n keys. * fix(cli): add the missing goal phase and paused status keys to uk, zh-CN and ko The i18n coverage guard fails because the Goals screen references four keys that the Ukrainian locale never defined: tui-goals-phase-mxit, tui-goals-phase-paus, tui-goals-phase-rlim and tui-goals-run-paused. Chinese and Korean carry the same gap, and the guard checks zh-CN too, so a Ukrainian-only fix would have moved the failure to the Chinese assertion. Ukrainian badges follow the file's truncation style (ПАУЗ from ПАУЗА, ЛІМІТ for the rate limiter, МАКС from МАКСИМУМ) and the paused status reads призупинено, matching the виконується/завершено/зупинено series. Chinese badges read 暂停, 限流 and 迭代上限 with the paused status 已暂停, reusing the terms the locale already uses for its run status strings. Korean keeps the English phase badges its five existing siblings use and translates the paused status as 일시 중지됨. The paused phase and status only render once #7973 lands, since GoalRunPhase::Paused and the pause/resume endpoints are not on main yet. * fix(cli): keep goal --watch alive through transient poll errors A single 5xx from GET /api/goals/{id}/run used to end --watch with exit(1): daemon_json surfaces the status error but still returns the body, which for a 500 with no JSON reads as running=false with an empty phase — the exit-code path then reported the goal as failed while the run kept going in the daemon. The long-form help promises the exit status reports the run's outcome, and a poll error is none of the four outcomes it lists. The poll is now gated on the HTTP status (daemon_json_checked, the helper added for the #6492 approvals false-success shape), and a poll the CLI cannot classify — failed status, or a 200 whose running=false carries no recognized terminal phase — is retried for up to five consecutive failures before the watch gives up with an explicit outcome-unknown message. Only a real terminal GoalRunPhase decides the exit code. Tests pin the classification: a 500/503 is unobservable, the four terminal phases are terminal, and a 200 with an unknown or missing phase is retried rather than reported as a failed run. * refactor(cli): spell the goal phase keys out instead of truncating them The phase keys were four-letter truncations (mxit, rlim, paus); the rendered values made the choice readable in the UI, but the key names are what a translator greps. Rename to match how the phase is spelled everywhere else (GoalRunPhase::MaxIterationsReached, max_iterations_reached): - tui-goals-phase-mxit -> tui-goals-phase-max-iterations - tui-goals-phase-rlim -> tui-goals-phase-rate-limited - tui-goals-phase-paus -> tui-goals-phase-paused The run-state keys already used the full spelling (tui-goals-run-max-iterations, tui-goals-run-paused), so the phase family now matches it. Mechanical rename, no behaviour change; the i18n coverage and dead-key tests gate every reference. --------- Co-authored-by: Evan <suzukaze.haduki@gmail.com>

…known phases honestly Address librefang#8067 review feedback: - Point RUN_PHASE_CONFIG at the existing, fully translated goals.run_phase_* keys and delete the parallel English-only goals.phase_* set from all five locale files. - Drop the paused phase: GoalRunPhase has no Paused variant, so the TS union keeps matching the wire; the paused badge arrives with librefang#7973. - Render an unknown phase as the raw value under the neutral badge variant instead of a confident Stopped, leaving the fallback label and icon unset. - Add Ban to the curated lucide barrel so the bundle links again. - Restore the runQuery polling comment and drop the run non-null assertions by narrowing with isRunning && run. - Remove the now-unreachable run_stop title branch and its dead goals.run_stop key from every locale. - Cover every API phase and the unknown-phase fallback with tests.

…and drop dead fallbacks Address librefang#8029 review feedback: - Drop the goal-status "paused" arms from progressForGoalStatus and goalStatusBadgeVariant: pause is a run-level phase (GoalRunPhase::Paused from librefang#7973), and GoalStatus never emits "paused", so those arms were unreachable and modelled the wrong concept. - Remove the dead defaultValue fallbacks from run_pause, run_resume and run_stop — all three keys exist and are translated in every locale, and a present default hides a future missing-key regression. - Add a translated run_phase_paused label to all five locales and a paused arm to the run-phase pill, which the stacked backend makes reachable. - Cover the pause and resume buttons with tests asserting the mutations fire with the goal id. - Add the changelog fragment. The branch is stacked on librefang#7973, which supplies POST /api/goals/{id}/pause and /resume and GoalRunPhase::Paused.

DaBlitzStein · 2026-09-03T05:28:31Z

Merge-order warning for this PR and #7785, found by merging every open branch into one integration tree — the two are compatible, but one specific conflict has a resolution that compiles fine and silently drops a feature.

Both branches touch run_loop and start_goal_run. That part is fine: git merges the signature additively, and kernel_api.rs comes out clean with start_goal_run extended and resume_goal_run intact. The features genuinely coexist.

The problem is the call site in routes/goals.rs. #7785 replaces the call with its own version carrying the loop-engineering parameters, and in doing so removes the require_paused branch this PR adds. Whoever resolves that conflict by taking one side wholesale loses either resume or loop-engineering, and the build stays green — there is no compile error and no failing test to catch it, because each side is internally consistent.

The resolution that keeps both:

let started = if require_paused {
    state.kernel.resume_goal_run(goal_id, agent_id, max_iterations)
} else {
    state.kernel.start_goal_run(goal_id, agent_id, max_iterations,
        loop_engineering, verify_agent_id, verify_max_retries, evaluator_model)
};

Two more things the second one to land will hit in the same area: the test modules in goal_runner.rs end up interleaved inside each other's bodies and have to be rebuilt from both branches rather than patched, and this PR's cadence test calls run_loop with the old signature, so it needs updating to the widened one.

Flagging it on both PRs. Nothing to change here — the note is for merge time.

DaBlitzStein · 2026-09-03T08:36:55Z

Pushed be55d2c5b: merged current main, which this branch was 67 commits behind.

No hand-written merge resolution to review — git merge-tree --write-tree <previous tip> origin/main against the committed tree reports zero lines of difference, so git's automerge and the committed result are identical.

Verification on the pushed tip: cargo check of the five touched crates — exit 0, 27 crates compiled. Scoped goal tests — 110 of 112 passing, and both of the remaining two green on a re-run at lower concurrency. goal_create_rejects_an_out_of_range_tick_interval failed at 40 s under load 18.8 and passes in 2.4 s at load 4; both failures carried Memory init failed: … database is locked at test-kernel boot, which is the connection pool timing out when the process cannot get scheduled — not a shared file, since mock_kernel puts its SQLite under its own tempdir.

Worth restating the merge-order note from earlier, since this refresh does not change it: this PR and #7785 both touch the run_loop / start_goal_run call site in routes/goals.rs, and resolving that conflict by taking one side wholesale silently drops either resume or loop-engineering while still compiling. The merge here came out clean only because #7785 is not in this tree — the collision appears when the two are combined.

DaBlitzStein · 2026-09-04T00:57:10Z

Completing the verification for 5ddf22d55, which was pushed with only the compile check.

cargo check -p librefang-api -p librefang-kernel -p librefang-types --all-targets — exit 0.
cargo nextest run -p librefang-api -E 'binary(goals_routes_integration)' — 50 tests run, 50 passed, 0 skipped, in 29.98 s.

No hand-written merge resolution: git diff between git's own automerge tree and this branch's HEAD is empty.

The 29.98 s is worth recording next to a number from earlier today, because it is the cleanest illustration of a false red this suite produces. The same binary took 792 s in a sibling worktree and reported 8 failures, all database is locked, all clustered at 31-34 s, with zero assertion failures. Re-run at one thread they passed in about 1.9 s each. Here, on an unloaded machine, the whole binary finishes in half a minute with everything green.

That is the honest way to read Memory init failed: … database is locked from mock_kernel: it is the connection pool's deadline expiring because the process cannot get scheduled, not a lock anyone holds — the test kernel builds its database in a fresh tempfile::tempdir() that no other test can even see.

Also worth stating: the conflict anticipated in routes/goals.rs between this branch and the loop-engineering one does not exist against main. It only appears when the two are merged with each other, where resume_goal_run has to forward the parameters start_goal_run gained rather than being filled with defaults — filling them compiles and silently drops the verifier on resume.

DaBlitzStein · 2026-09-07T11:50:22Z

Addressed at 3e5202001, all four points.

1 — the resume keeps the paused run's cap. GoalRunner::start resolves max_iterations itself now, in the order: explicit argument, then the checkpoint's own cap, then the compiled default. It is resolved there because that is the only layer that knows whether this is a resume — by the time goal_run_start sees a None it cannot tell it apart from an operator asking for the default.

On your explicit question about what an explicit cap on a resume body should mean: it is honoured, and the doc comment says so. A resume naming max_iterations is a deliberate re-budgeting of the remaining run, and refusing it would leave no way to extend a run about to hit its cap.

The discriminator you asked for is goal_run_resume_keeps_the_paused_runs_iteration_cap, driven through POST /resume. Dropping the checkpoint fallback fails it with your scenario's numbers:

assertion `left == right` failed: an absent cap must restore the paused run's,
not the compiled default
  left: 25
 right: 100

an_explicit_cap_outranks_the_checkpoints_on_resume and goal_run_resume_rejects_an_invalid_iteration_cap cover the other two directions.

2 — paused_at is read now, and started_at survives. ResumePoint carries both, state() uses paused_at for updated_at and the checkpoint's started_at for started_at, so two consecutive GET /run calls on a motionless goal return the same timestamps and "paused for how long" is answerable. start() restores the original started_at rather than re-stamping: a resume continues the run it checkpointed rather than opening a new segment, and that is the documented choice rather than a side effect.

3 — two changelog fragments, pause/resume and the cadence control separately, as you suggested.

4 — display_name is gone from goal_tick_sender_context; the constant is used directly.

27 kernel tests green, including the four new ones and the five HTTP cases.

Ready for another look.

DaBlitzStein · 2026-09-07T15:32:00Z

Cross-reference found while building the integration branch (main + all 41 open PRs). This PR and #7785 cannot both merge as they stand, and neither is wrong on its own — which is why both are green.

Both change the same two functions in crates/librefang-kernel/src/goal_runner.rs, in ways that do not compose:

	`GoalRunner::start`	`run_loop`
`main` (`95c258cc4`)	`pub fn start<F, Fut>(`	`async fn run_loop<F, Fut>(`
#7785 (loop engineering)	`pub fn start<F, Fut, L, E, Efut>(` + `on_learnings_captured`, `evaluate_goal`, `loop_engineering`, `verify_agent_id`, `verify_max_retries`, `evaluator_model`	`<F, Fut, L, E, Efut>` with the same six
this PR (pause/resume)	keeps `<F, Fut>`, resolves `max_iterations` internally from the pause checkpoint, and takes `started_at` from it	adds the `pause: Arc<AtomicBool>` flag

Whichever lands second does not compile: the first's call sites pass an argument list the second's signature does not have, and every run_loop call site in the test module has the same problem from the other direction.

The reconciliation is small and I have already validated it, on an integration branch that carries both:

Keep feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785's generics and #[allow(clippy::too_many_arguments)], and feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785's parameter list.
Keep this PR's body wholesale — the checkpoint read before stop_locked, the max_iterations precedence (explicit → checkpoint → compiled default), and started_at: resume.as_ref().and_then(|r| r.started_at).unwrap_or(now).
run_loop takes both stop and pause, and feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785's six loop-engineering parameters.
GoalRunState's reconstruction from a checkpoint in state() needs verify_agent_id / verify_max_retries / evaluator_model, which the checkpoint does not store. Reading them back from the goal document — the same place start's caller reads them from — keeps a paused run reporting the verifier its resume will actually use, instead of a blank that reads as "no gate on this run".
KernelApi::resume_goal_run and goal_run_resume take the four loop-engineering arguments too, so /resume does not silently drop a verifier the operator configured.

With that in place, both feature sets pass together: 65 kernel goal tests green, including verifier_rejection_sends_the_work_back_to_the_generator (#7785) alongside pause_checkpoints_and_start_resumes_from_it, an_explicit_cap_outranks_the_checkpoints_on_resume and run_loop_waits_the_goals_configured_tick_interval (this PR).

What I am not doing unilaterally: rebasing one of these onto the other. That decides the merge order for two of your PRs and turns whichever loses into a stacked branch that cannot merge alone. Tell me which one should go first and I will push the rebase with the reconciliation above the same day — it is one function pair plus the test call sites, and I have the resolved version ready.

DaBlitzStein · 2026-09-07T15:54:34Z

From a surface-parity audit of all 41 open PRs against CLAUDE.md's rule that the TUI and the WebUI are as mandatory as the API, never a follow-up.

tick_interval_secs reaches no surface at all. This branch adds it to the goal document (librefang-types/src/goal.rs) with MIN / MAX / DEFAULT constants and validates it at the API boundary (routes/goals.rs), and the runner honours it — run_loop_waits_the_goals_configured_tick_interval pins that. But grepping all 41 PR heads for tick_interval_secs under crates/librefang-api/dashboard and crates/librefang-cli returns zero files: not the dashboard, not the TUI, not the CLI, in any open branch.

It is also not a KernelConfig field, so it never appears in kernel_config_schema.golden.json and the schema-driven editors (#8184's TUI config screen, the dashboard's ConfigPage) cannot pick it up for free the way they do for tool_exec.default_timeout_secs or skills.promotion.*. The only way to set it is to hand-edit the goal's JSON.

A validated knob with bounds nobody can reach is the shape this rule exists to prevent. The cheapest honest fix is a field on the goal create/edit form in the dashboard plus the matching TUI input, but I would rather ask than guess: do you want that in this PR, or split out with this one noting the gap?

Not blocking anything I can see, and the backend half is sound.

DaBlitzStein · 2026-09-07T15:58:24Z

Addendum to the collision note above: it is a three-way, not a two-way. #8029 carries its own copy of this backend, and it is a strict subset of this one — start and run_loop unchanged from main, and resume_goal_run at routes/goals.rs:206-211 taking no body and passing None where this PR's :209-214 forwards body: Option<Json<Value>>.

The routes themselves are safe: both register byte-identical .route("/goals/{id}/pause") and .route("/goals/{id}/resume") lines that collapse into one registration. What does not survive a careless resolution is the re-budgeting your approval singled out — resolve the handler bodies toward #8029 and a resume silently falls back to the compiled default cap, with an_explicit_cap_outranks_the_checkpoints_on_resume on the other side of the file that was replaced.

So the order decision for #7785 and this PR should cover #8029 too. Noted there as well.

DaBlitzStein · 2026-09-07T17:32:51Z

Stacked on #7785 and pushed at 19eef8698, which resolves the GoalRunner collision from this side. The order picked is #7785 → this PR → #8029.

The reconciled signature. GoalRunner::start and run_loop are now <F, Fut, L, E, Efut> — #7785's generics and its six loop-engineering parameters — carrying this PR's body wholesale: the checkpoint read before stop_locked, the max_iterations precedence (explicit → checkpoint → compiled default), and started_at: resume.as_ref().and_then(|r| r.started_at).unwrap_or(now). run_loop takes both stop and pause.

Three things needed a decision rather than a merge, and each is small:

GoalRunner::state() reconstructing a paused run from its checkpoint now needs verify_agent_id / verify_max_retries / evaluator_model, which the checkpoint does not store. They come back from the goal document — the same place start's caller reads them from — so a paused run reports the verifier its resume will actually use rather than a blank that reads as "no gate on this run". Extending the stored checkpoint would have been a schema change this PR has no reason to make.
goal_run_resume and KernelApi::resume_goal_run take the four loop-engineering arguments too. A resume that dropped them would silently lose the gate the operator configured on the goal, which is the same class of silent loss as the cap this PR exists to preserve.
KernelApi::start_goal_run keeps feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785's wide signature, since it is the layer feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785 widened.

Verification, on the stacked tree:

cargo test -p librefang-kernel --lib goal → 65 passed, 0 failed, with both feature sets in one binary: verifier_rejection_sends_the_work_back_to_the_generator, verifier_pass_lets_goal_done_finish_the_run and the_evaluator_can_finish_a_goal_the_agent_never_claimed from feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785, alongside pause_checkpoints_and_start_resumes_from_it, an_explicit_cap_outranks_the_checkpoints_on_resume, resume_restores_the_checkpoints_cap_and_start_time and run_loop_waits_the_goals_configured_tick_interval from here.
cargo test -p librefang-api --test goals_routes_integration → 65 passed.
cargo clippy -p librefang-kernel -p librefang-api --all-targets -- -D warnings → clean.

What is deliberately not here: I lifted the reconciliation from an integration branch that also carries #7781, and kernel_api.rs came across with #7781's set_agent_model_routing attached. That is stripped — grep -c "set_agent_model_routing\|model_router\|AgentRouterOverride" on kernel_api.rs is 0. Nothing from any other open PR is in this diff.

#8029 is the remaining leg. It carries its own strict subset of this backend, and once this lands it should keep only its dashboard layer — which is what its title says it is. Noted there.

DaBlitzStein · 2026-09-07T19:10:01Z

The stacking commit I pushed earlier did not build. Fixed at bb3127819; the branch is also refreshed onto current origin/main.

19eef8698 widened goal_run_start on the kernel and resume_goal_run on the KernelApi trait, and left start_goal_run on the trait at its three-argument shape. That leaves two call sites disagreeing in opposite directions:

kernel_api.rs — the trait impl calls the kernel's seven-argument goal_run_start with three
goal_runner.rs:188 — calls the trait's three-argument start_goal_run with seven

→ two E0061s, could not compile librefang-kernel.

How it got past me, since that is the part worth recording. The edit existed and was verified — cargo check -p librefang-kernel -p librefang-api --all-targets clean, 65 goal tests green, clippy -D warnings clean. All of it ran against a working tree that contained the fix, and the fix never got staged into the commit. What I tested and what I pushed were not the same tree, and every signal I had was about the tree I tested.

The corrected commit is verified the same way, but this time against what is actually committed: cargo check -p librefang-kernel -p librefang-api --all-targets clean, cargo test -p librefang-kernel --lib goal 65 passed.

I then audited the other nine branches I pushed today for the same shape — working tree versus committed HEAD versus pushed remote, per branch. All nine identical on all three. This was the only one.

Nothing about the reconciliation itself changed: start/run_loop keep <F, Fut, L, E, Efut> with #7785's six loop-engineering parameters and this PR's resume body, and the three decisions I described above still hold. The only difference is that the trait now says what the kernel already said.

Separately, and thanks to a review pass: the comment on GoalRunner::state() claimed all three loop-engineering fields come back from the goal document. Two do; verify_max_retries is not a Goal field at all — it arrives through /start's optional body and neither the document nor ResumePoint persists it, so the reconstruction uses the compiled default. That is still the right value to report, because a resume without an explicit number runs under the same default, but the comment sent the reader hunting for a field that does not exist. Corrected in the integration branch and it will come across on the next push here if you want it in this PR — say the word and I will cherry-pick it.

DaBlitzStein · 2026-09-13T20:12:42Z

Rebasado sobre 57ad92341 (origin/main, release v2026.9.14). Nueva punta: ed8fa353d.

Resoluciones no triviales:

GoalsPage.tsx — reestructuración en pestañas (fix(dashboard): let the Goals page create a goal when there are none #8251, cd140da8e) contra los controles de loop engineering.
Main traía la página partida en dos paneles, el formulario de creación dentro del panel goals, ref={createTitleRef} y el botón de crear en la cabecera; la rama traía los selectores de agente y verificador, el checkbox de loop engineering, el input de modelo evaluador y el badge de fila, escritos contra la estructura plana anterior.
Se conserva la estructura de main y se reinjertan los controles de la rama en su nueva anidación.
GoalsPage.tsx — case "paused" traducido al vocabulario de feat(dashboard): display goal run phase badges #8067.
HEAD traía goalRunPhaseBadge devolviendo {variant: BadgeVariant, icon}; la rama traía case "paused" escrito contra el {bg,text,dot} anterior.
La propia rama ya había resuelto exactamente esta colisión en su merge 94838a130, así que se reproduce esa resolución literal — case "paused": return { variant: "warning", icon: Pause };, con su comentario de dos líneas y Pause añadido al import de lucide-react — en vez de inventar una nueva.
routes/goals.rs — la llamada de arranque movida detrás de la lectura de configuración.
El commit 9cdaea90e mueve start_goal_run para que se ejecute después de resolver loop_engineering / verificador / evaluador desde el documento del objetivo, porque ahora los necesita como argumentos.
Ese commit fue escrito cuando solo existía start_goal_run; aquí ya existe la bifurcación if require_paused { resume_goal_run(...) } else { start_goal_run(...) } de pausa/reanudación, así que se conserva la bifurcación y se le pasan los siete argumentos a las dos ramas.
kernel_api.rs, goal_lifecycle.rs, goal_runner.rs, goal.rs — lo que el rebase se comió, y el porqué.
Esta rama llevaba su integración en commits de merge (19eef8698 merge: stack on #7785, 94838a130 Merge origin/main).
git rebase linealiza y descarta los merges, así que las resoluciones que vivían solo ahí desaparecen sin que ningún conflicto lo señale.
Lo perdido y restaurado en ed8fa353d:
- goal_run_resume había perdido sus cuatro argumentos de loop engineering tanto en el método inherente del kernel como en el trait KernelApi, mientras routes/goals.rs seguía llamándolo con los siete. Eso no compila.
- El checkpoint de pausa había dejado de reconstruir verify_agent_id, verify_max_retries y evaluator_model desde el documento del objetivo, con lo que una ejecución pausada reportaba verificador en blanco.
  El criterio de la restauración es comprobable: todo fichero que main no ha tocado desde la base de fusión vuelve al contenido que ya tenía la punta de la rama, que para esos ficheros es por construcción el resultado correcto del rebase.
  kernel_api.rs sí lo tocó main, así que ahí se parte de la versión de main y se recupera solo la declaración y la implementación ensanchadas de resume_goal_run.

Verificado (dashboard, con LANG=en_US.UTF-8):

Conjunto de ficheros idéntico antes y después del rebase: 38 ficheros.
Los cinco locales/*.json: JSON válido, 0 duplicadas (con object_pairs_hook), 0 claves muertas resucitadas, las 9 claves de la rama presentes en los cinco idiomas.
Coherencia de aridad entre las tres capas: goal_lifecycle.rs, kernel_api.rs y routes/goals.rs coinciden con la punta original.
npx tsc --noEmit: 0 errores. npx eslint .: 0. npx pnpm build: OK.
npx vitest run: Test Files 1 failed | 209 passed (210), Tests 1 failed | 1929 passed (1930).
El único rojo es PromptsExperimentsModal / "stops selection when every variant has a traffic bucket" (timeout de 5 s), ajeno a este PR.

NO verificado: la mitad Rust (librefang-kernel, librefang-cli, librefang-api, librefang-types) no se ha compilado ni testeado; no se ejecutó cargo de ningún tipo.
Dado que la restauración descrita arriba es precisamente de código Rust, conviene que la comprobación de la integración empiece por ahí.

Revisiones sin atender: quedan 9 hilos de revisión sin resolver, 2 de ellos todavía vigentes.
Este rebase no los aborda.

librefang#7973 carried its own copy of the loop-engineering work, frozen at the point librefang#7785 had reached when this branch was cut (`43b4ce22b`, "docs(goals): say what an unresolvable evaluator_model does"). Everything librefang#7785 added after that — the verifier-gate back doors, the `goal_update` status/progress bypass, the per-leg circuit breakers, and the operator stop interlock — was missing here, and integrating the two branches side by side would have resolved the same region twice. Merging `feat/loopeng` makes it an ancestor, so this PR's own diff is now just pause, resume and the configurable tick cadence. Shared loop-engineering code takes librefang#7785's newer form: - `stop` becomes `Arc<StopFlag>` (who raised the stop, not just that it is up), so a `PUT /api/goals/{id}` that writes a terminal status is not reverted by the iteration in flight while a bare `POST /stop` still lands its accounting. - `evaluator_goal_statement` gates the completion judge on a non-empty goal. - `GateStreak` gives each verifier leg its own circuit breaker. - `optional_uuid_field` handles `verify_agent_id`, and an agent may no longer be its own verifier (create and update, checked on the effective post-update pair). This branch's own fixes are grafted on top rather than reverted to librefang#7785's shape: - `max_iterations` stays an `Option` down to `GoalRunner::start`, so a resume does not overwrite the cap the paused run was under. librefang#7785's `unwrap_or(DEFAULT_GOAL_MAX_ITERATIONS)` in `goal_run_start` stays removed. - `resolve_workshop_config` keeps the skill-workshop opt-in gate. - `goal_tick_sender_context` keeps the per-goal `chat_id` session scoping. - `pause: Arc<AtomicBool>` and `PAUSE_POLL_INTERVAL` are added alongside the new `StopFlag`. Two collisions git merged without a marker, both removed by hand: - `goal_run_start_rejects_invalid_verify_max_retries` was defined by both sides in `goals_routes_integration.rs`. Kept librefang#7785's, which also covers a negative value; kept this branch's doc comment. - `progress_100_without_goal_done_still_finishes_with_no_verifier_configured` was defined twice in `goal_runner.rs`. Kept librefang#7785's, which asserts the turn count and the stored progress rather than only the phase. Also drops a stale second `Optional body:` line from `start_goal_run`'s doc comment, which contradicted the one above it by omitting `verify_max_retries`.

Nine commits behind, which put its SCHEMA_VERSION at 57 against main's 60 and left the merge base far enough back that stacking librefang#7973 on top would have re-resolved the whole shared goals region. Clean auto-merge; the only files touched on both sides are the dashboard's `api.ts` and `lib/http/client.ts`.

This branch carried its own copies of both the pause/resume kernel work (librefang#7973) and the loop-engineering work (librefang#7785), each frozen at a different point, so integrating it beside them re-resolved the same 4 000-line region three times. It also sat nine commits behind main, at `SCHEMA_VERSION` 57 against main's 60; the preceding commit merges main, this one merges librefang#7973, which already contains librefang#7785. Its own diff is now the dashboard's pause/resume controls plus the resume 404. The kernel and API files came from librefang#7973 wholesale where this branch adds nothing to them (`goal_lifecycle.rs`, `kernel_api.rs`, `kernel/tests.rs`) and grafted where it does: - `goal_runner.rs` takes librefang#7973's version, then this branch's own fix that a paused readout reports the checkpoint's `verify_max_retries` rather than the compiled default. The reconstruction still rebuilds all three fields from the goal document: `verify_agent_id` and `evaluator_model` are read from the goal (gated on `loop_engineering`), and only the retry budget comes from the checkpoint, which is its only record. - `routes/goals.rs` keeps the resume precondition ordering — the goal lookup runs before the `require_paused` check, so an unknown id answers 404 and not a 409 that advises a `/start` which would 404 anyway. - `GoalsPage.tsx` keeps this branch's pause/resume/stop cluster and takes librefang#7785's verifier-select filtering (an agent is not offered as its own verifier, and reassigning the goal to its verifier clears the verifier). - `GoalsPage.test.tsx` keeps this branch's `quiescing` example for the unknown-phase badge; librefang#7973 used `awaiting_review`, which a later PR could make a known phase and quietly void the test. `goal_run_resume` keeps its seven parameters (`goal_id`, `agent_id`, `max_iterations`, `loop_engineering`, `verify_agent_id`, `verify_max_retries`, `evaluator_model`), matching the call in `routes/goals.rs`. The one line of librefang#7973 deliberately not taken is its Korean `run_phase_paused` ("일시중지"); this branch's own i18n commit spells it "일시 중지됨".

…ibrefang#7785 This branch carried its own copies of the pause/resume kernel work (librefang#7973) and the loop-engineering work (librefang#7785), each older than the versions now on those branches, and it never had librefang#7973's "cancel discards a pause checkpoint it cannot read". Merging librefang#8029 — which already contains librefang#7973 and librefang#7785 — leaves this PR's own diff at the TUI: the `[p]` pause/resume binding on the Goals screen, the run phase surviving a list refresh, and the Stop button's tooltip. The split is by file, because the two sides do not overlap: - Everything under `crates/librefang-api/` and `crates/librefang-kernel/` comes from librefang#8029. This branch added nothing there that librefang#8029 does not already have in a newer form — its `goal_runner.rs` still had `stop: Arc<AtomicBool>` instead of `StopFlag`, `stop_locked` without `stopper_wrote_goal`, the shared `goal_learnings_<id>` key instead of the per-run one, and the conditional `clear_pause_checkpoint` that left an unreadable checkpoint behind. - Everything under `crates/librefang-cli/` stays as this branch wrote it. `tui/mod.rs` keeps `replace_goals(list)` rather than librefang#8029's inherited `goals = list; refilter()` — the whole point of `38bdac968` is that the plain assignment wiped the run phase a concurrent `/run` fetch had just written; `replace_goals` carries `run_phase`, `run_iteration` and `run_max_iterations` across the refresh and still calls `refilter()`. The four `tui-goals-hints` lines are this branch's, not librefang#8029's: they are the same bar with `[p] Pause/resume` folded in, so no shortcut is lost and no Fluent id is defined twice (checked in all four locales — a duplicate id makes `i18n::init` panic on the whole bundle). All 1 017 Fluent ids the TUI references resolve in en, ko, uk and zh-CN. Verified: `npx tsc --noEmit` clean, `npx eslint .` clean, `npx vitest run` 209/210 files and 1 932/1 933 tests green (the one failure is the unrelated `PromptsExperimentsModal > stops selection when every variant has a traffic bucket` timeout, which fails on main too), `pnpm build` clean, and `rustfmt --check` clean on every `.rs` touched. The Rust build and test suites were not run here — the shared target directory is in use by other work; CI is the gate.

This branch only ever needed librefang#7785 — it has no pause/resume — but it carried librefang#7785 frozen at `43b4ce22b`, before the verifier-gate back doors, the `goal_update` bypass, the per-leg circuit breakers and the operator stop interlock were closed. Placing it above librefang#8224 rather than directly above librefang#7785 keeps the stack linear and confines the "carries work it does not depend on" cost to this branch instead of pushing the TUI loop-engineering screen down into librefang#7973, librefang#8029 and librefang#8224, none of which need it. The merge used `43b4ce22b` as its base, which is exact: that commit and this branch's `278254748` have byte-identical trees, so the three-way saw only the two sides' real additions rather than two copies of librefang#7785. Two defects the merge itself created, both fixed here: - `GoalsState::replace_goals` (librefang#8224's fix for a list refresh wiping the run phase) did not carry `run_verify_max_retries`, the field this branch adds and which the same `apply_run_state` writes. A list refresh landing after a `/run` fetch blanked it, and `adjust_verify_max_retries` then started `+`/`-` from the compiled default instead of the budget the live run is under — the operator would watch a number they were not shown move. `a_list_reload_keeps_run_state_already_fetched_so_pause_stays_live` now asserts the budget survives the reload, and `a_goal_absent_from_the_reload_does_not_carry_its_run_state_over` asserts it is not carried onto a different goal. - librefang#8224's two `apply_run_state` call sites in that file passed four arguments; this branch widened the method to five. Neither branch could see the other, so both were correct alone and the pair does not compile. Where the two sides changed the same thing, this branch's newer form wins: `toggle_run` as `&self`, the named `STEP_*` wizard indices in place of the numeric ones and `create_visible_steps()` in place of the fixed `CREATE_STEPS`, the four-argument `spawn_start_goal_run` carrying `verify_max_retries`, the multi-row hint `Paragraph` in place of the single clipped `hint_bar`, and the shortened Ukrainian run hints. librefang#8224's `[p]` binding, `toggle_pause`, `is_paused` and `replace_goals` are kept whole, in both the list and the detail key handlers. The `tui-goals-hints` bar is librefang#8224's, with `[p] Pause/resume`; this branch never touched that line — its own `[+/-]` hint is a separate row rendered only when the open goal uses loop engineering, because that pane is 39 columns wide on an 80-column terminal and an appended hint is clipped. No Fluent id is defined twice in any of the four locales. Verified: `npx tsc --noEmit` clean, `npx eslint .` clean, `npx vitest run` 209/210 files and 1 932/1 933 tests green (the one failure is the unrelated `PromptsExperimentsModal > stops selection when every variant has a traffic bucket` timeout), `pnpm build` clean, `rustfmt --check` clean on every `.rs` touched. The Rust build and test suites were not run here; CI is the gate.

This branch carried its own copies of librefang#7973's pause/resume kernel work and librefang#7785's loop engineering, both older than the versions those branches now carry, and it never had librefang#7973's "cancel discards a pause checkpoint it cannot read". Merging librefang#8229 — which contains librefang#8224, librefang#8029, librefang#7973 and librefang#7785 — leaves this PR's own diff at what it is for: showing and setting the goal runner's tick interval on the TUI and the dashboard, the two surfaces librefang#7973 left without one. The merge used `751e31b5b` as its base. That commit is a real ancestor of librefang#8229 and its tree is byte-identical to this branch's `dd7843db5`, so the three-way saw the two sides' real additions instead of two copies of the shared goals work. Kernel and API files come from librefang#8229 wholesale: this branch adds nothing to `goal_runner.rs`, `goal_lifecycle.rs`, `kernel_api.rs` or `routes/goals.rs` that librefang#8229 does not already have in a newer form. Its `goal_runner.rs` still had `stop: Arc<AtomicBool>`, the shared `goal_learnings_<id>` key, and the conditional `clear_pause_checkpoint` that left an unreadable checkpoint behind. The real work was the TUI create wizard, which both branches extended with a step of their own. The combined order is title, description, agent, tick interval, loop engineering, verifier, evaluator: - The cadence step sits *before* the loop-engineering toggle on purpose. `create_visible_steps()` cuts the wizard at the toggle when loop engineering is off, so a cadence step placed after it would be unreachable for every plain goal — which is most of them. - `CREATE_STEPS` is 7 and `CREATE_STEPS_PLAIN` is 5. - The submit handler runs both halves: this branch's `parsed_create_tick_interval()` still refuses an out-of-range cadence at the keyboard with the bounds message, and librefang#8229's `configured` guard still drops a verifier / evaluator typed before the toggle was turned back off. - The `Char` and `Backspace` handlers keep this branch's `status_msg.clear()` and digits-only cadence filter alongside librefang#8229's boolean-toggle and verifier / evaluator arms. - The info-hint line uses this branch's `t_args` form with the cadence bounds; librefang#8229's three new hints have no placeholders and ignore them. - The wizard's status row (this branch's `chunks[8]`) and librefang#8229's nav-hint row now both render — the nav bar moved to `chunks[9]`. Tests that pinned a numeric step were retargeted rather than deleted: `s.create_step = CREATE_STEPS - 1` no longer means the cadence step, so the four cadence tests take `STEP_TICK_INTERVAL`, and the two that must reach the submitting step take `STEP_LOOP_ENGINEERING`. `an_out_of_range_tick_interval_blocks_submission_and_explains` gained an explicit walk from the cadence step to the submitting one, because the value is refused at submit time and not on leaving its own field. Also removed two leftovers the three-way produced without a marker: a second copy of the pre-merge submit handler after the resolved one, and a second `_ =>` arm in the wizard's field-render match that was both unreachable and typed `&String` where the others are `String`. Verified: `npx tsc --noEmit` clean, `npx eslint .` clean, `npx vitest run` 210/211 files and 1 948/1 949 tests green (the one failure is the unrelated `PromptsExperimentsModal > stops selection when every variant has a traffic bucket` timeout), `pnpm build` clean, `rustfmt --check` clean on every `.rs` touched, no duplicate Fluent id in the four CLI locales and no duplicate key in the five dashboard locale JSONs (`object_pairs_hook`). The Rust build and test suites were not run here; CI is the gate.

DaBlitzStein · 2026-09-13T22:03:46Z

Ahora se apila sobre #7785 (`2adb4cba4`)

Esta rama llevaba su propia copia del trabajo de loop engineering, congelada en el punto al que #7785 había llegado cuando se cortó la rama: 43b4ce22b, «docs(goals): say what an unresolvable evaluator_model does».
Todo lo que #7785 añadió después faltaba aquí, así que integrar las dos en paralelo resolvía la misma región dos veces y con resultados distintos.

Sobre qué se apila y por qué: #7785 va debajo porque es la copia más nueva del trabajo compartido — 977 líneas suyas faltaban en esta rama — y porque no necesita nada de nadie.
Esta rama es la segunda de la pila porque solo necesita a #7785, y porque es la que #8029, #8224 y #8230 necesitan a su vez (les faltaban 65, 80 y 85 líneas suyas respectivamente).

La fusión no usó la base que git elegía por defecto.
Los árboles de 43b4ce22b (#7785) y 57c368907 (la copia de esta rama) difieren solo en lo que esta rama añade por debajo, así que apliqué exactamente 43b4ce22b..0734e56f0 — los siete commits nuevos de #7785 — sobre este árbol.
Eso redujo el conflicto de nueve ficheros a cinco, y esos cinco solo donde los arreglos tardíos de las dos ramas tocan las mismas líneas.

Qué se conservó de cada lado. Del lado de #7785, por ser la versión más nueva del trabajo compartido:

stop pasa a Arc<StopFlag> — quién levantó la parada, no solo que está levantada — para que un PUT /api/goals/{id} con estado terminal no lo revierta la iteración en vuelo, y para que un POST /stop a secas siga guardando su contabilidad.
evaluator_goal_statement impide llamar al juez de compleción con un objetivo vacío.
GateStreak da a cada pata del verificador su propio cortacircuitos.
optional_uuid_field para verify_agent_id, y un agente ya no puede ser su propio verificador (en creación y en actualización, comprobado sobre el par efectivo tras la escritura).

De este lado, injertado encima en vez de revertido a la forma de #7785:

max_iterations sigue siendo Option hasta GoalRunner::start, para que un resume no pise el tope bajo el que estaba la ejecución pausada. El unwrap_or(DEFAULT_GOAL_MAX_ITERATIONS) que feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785 tiene en goal_run_start sigue eliminado.
resolve_workshop_config mantiene la puerta de opt-in del taller de skills.
goal_tick_sender_context mantiene el chat_id por objetivo que separa las sesiones.
pause: Arc<AtomicBool> y PAUSE_POLL_INTERVAL conviven con el nuevo StopFlag.

goal_run_resume termina con sus siete parámetros — goal_id, agent_id, max_iterations, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model — que es como lo llama crates/librefang-api/src/routes/goals.rs.

Dos colisiones que git fusionó sin dejar marcador, quitadas a mano:

goal_run_start_rejects_invalid_verify_max_retries quedaba definido dos veces en goals_routes_integration.rs. Me quedé con el de feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785, que además cubre un valor negativo, y conservé el comentario de doc de esta rama.
progress_100_without_goal_done_still_finishes_with_no_verifier_configured quedaba definido dos veces en goal_runner.rs. Me quedé con el de feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785, que afirma sobre el número de turnos y el progreso persistido en vez de solo sobre la fase.

Invariante. Para cada fichero que toca #7785, líneas suyas ausentes del resultado: 76, en 6 ficheros, todas justificadas una a una en el mensaje del commit de fusión.
Son formas antiguas que esta rama sustituye deliberadamente — el TICK_INTERVAL fijo frente a la cadencia configurable, la unión de fases sin paused, la firma de 3 argumentos de GoalRunner::new_with_store, el SenderContext en línea que aquí es goal_tick_sender_context, el unwrap_or(DEFAULT_GOAL_MAX_ITERATIONS) — o líneas idénticas con otra indentación.
Contra el tip anterior de esta misma rama: 119 líneas, todas sustituciones por la versión más nueva de #7785, más las 6 del test duplicado que se quitó.

También he quitado una segunda línea Optional body: del doc comment de start_goal_run, que contradecía a la de arriba omitiendo verify_max_retries; ya estaba así antes de la fusión.

Verificado: npx tsc --noEmit limpio, npx eslint . limpio, npx vitest run 209/210 ficheros y 1.930/1.931 tests en verde, pnpm build limpio, rustfmt --check limpio en todos los .rs tocados.
El único test rojo es PromptsExperimentsModal, el caso del bucket de tráfico: es ajeno a este cambio y falla igual en main.

No he compilado ni ejecutado la parte Rust: el target compartido está en uso por otro trabajo. CI es la puerta.

El push fue avance rápido — el tip anterior ed8fa353d sigue siendo ancestro de 2adb4cba4 — así que no se reescribió historia y ningún comentario de revisión quedó huérfano.

Revisiones: los 9 hilos en línea tienen respuesta de @DaBlitzStein pero ninguno está marcado como resuelto; la última revisión de @houko es APPROVED (2026-09-12). No he tocado ni cerrado ningún hilo.

… run_loop `run_loop` went from twelve parameters to fourteen here — `pause` in position eleven, `initial_learnings` in fourteen — and `stop` from `Arc<AtomicBool>` to `Arc<StopFlag>`. Sixteen call sites in the test module were left on the old shape and five still passed an `AtomicBool` where the flag now goes, so `cargo check -p librefang-kernel --all-targets` never compiled on this branch. The pause slice loop had the matching miss: `stop.load(Ordering::SeqCst)` on a `StopFlag`, which has no `load`. It reads `stop.is_raised()` like the other check at the top of the loop. Not caught earlier because a scoped `cargo check -p librefang-kernel` without `--all-targets` builds the lib alone and never looks at the test module, and the lib error sits behind it. `cargo check -p librefang-kernel --all-targets`: 0 errors.

DaBlitzStein · 2026-09-13T22:55:08Z

Esta rama no compilaba: arreglado en `8fbb34da5`

Al ampliar run_loop de doce a catorce parámetros (pause en la posición once, initial_learnings en la catorce) y cambiar stop de Arc<AtomicBool> a Arc<StopFlag>, se quedaron sin actualizar:

16 llamadas del módulo de test con la firma antigua de doce argumentos.
5 llamadas que siguen pasando Arc::new(AtomicBool::new(false)) donde ahora va el flag.
1 sitio en el bucle de pausa: stop.load(Ordering::SeqCst) sobre un StopFlag, que no tiene load. Ahora es stop.is_raised(), igual que la comprobación de la cabecera del bucle.

Por qué no saltó antes: un cargo check -p librefang-kernel sin --all-targets compila solo la lib y no mira el módulo de test, y el error de la lib queda por delante tapando el resto.

cargo check -p librefang-kernel --all-targets: 0 errores.

`learnings_survive_a_pause_and_resume_cycle` looked for `goal_learnings_{goal_id}`, a key nothing writes. librefang#7785's review moved lessons to a per-run key so a second run of the same goal cannot overwrite the first run's — `structured_set` replaces the document — and this test was written against the older per-goal shape, so it failed on `expect("learnings must be persisted")` before reaching a single one of its own assertions. Both halves of the cycle do still land under one key, which is what the test is really about: a resume continues the run it checkpointed and keeps its original `started_at` instead of stamping a new one, so `run_loop` derives the same identity either side of the pause. The checkpoint is where that identity can be read without racing — the registry entry `runner.state()` would serve it from self-cleans the moment `run_loop` returns, and the test has already waited for exactly that. Verified it discriminates rather than merely passing: with `resume_learnings` replaced by an empty vector at the `run_loop` call site, it fails with `a pause must not discard learnings captured before it: ["learned after the resume"]`. Restored, 65 `goal_runner` tests pass.

…#7785) * feat(goals): gate autonomous runs on a verifier and an evaluator A goal run ended the moment the agent wrote GOAL_DONE, which made the worker the sole judge of its own work - the one check a long-horizon loop most needs, and the one it did not have. Setting loop_engineering on a goal adds two judges that are not the worker. A verifier agent reads each iteration's output and returns VERDICT: PASS|FAIL|NEEDS_REWORK; a rejection goes back to the generator carrying the verifier's stated reason, up to verify_max_retries rework rounds, and until the verifier passes the work GOAL_DONE does not end the run. An evaluator model makes one cheap yes/no read of the goal against the latest output and can conclude the goal is met even when the agent never claimed it. The agent can also record a reusable lesson with GOAL_LEARNED: <text>. Captured lessons are replayed into later iterations' prompts, persisted under goal_learnings_<id>, and written into a goal-learned-* skill through the same prompt-injection scan every other skill-creation path uses. All three are inert unless the goal opts in: with loop_engineering off the prompt is byte-identical to the one it was and the run still makes exactly one LLM call per iteration. a_plain_run_makes_no_extra_llm_calls asserts that, because an evaluator that fires on every goal would double every existing operator's bill silently. Sub-agents are delegated, not provisioned. The prompt directs the agent to its own agent_spawn / agent_send tools, which run under the capabilities its operator granted; neither the runner nor the API ever creates an agent on a caller's behalf. loop_engineering_without_a_verifier_provisions_no_agent and goal_run_start_with_an_unusable_agent_id_provisions_no_agent assert the registry does not grow, which is the property that survives a later refactor - asserting only the status code would not. An unusable verify_agent_id is a 400 with the id named rather than a silently ignored field, because dropping it downgrades a gated run to an ungated one without telling the operator who configured the gate. A run now also gives up after five consecutive non-rate-limit tick failures instead of spending its whole iteration budget rediscovering a deleted agent or a revoked key. Refs #6505 * fix(goals): register a goal run's handle before spawning its loop start() spawned the loop task and inserted its RunHandle afterwards. A loop that finishes inside that window runs its self-cleanup remove_if against a registry that does not hold it yet: the removal finds nothing, the insert then lands a handle for a run that is already over, and nothing ever collects it. state() reports the run as present forever and the registry grows by one every time it happens. The window is short, but the exits that fit inside it are the fast ones - a pre-signalled shutdown, or a goal deleted between the API's read and this call - both of which end the loop before its first agent turn. The handle is now registered before the spawn and the JoinHandle backfilled after. A missing entry at backfill time means the loop already finished and cleaned up, so dropping the handle there is correct: there is nothing left to abort. The generation check keeps a replacement run's handle from being overwritten, and stop() cannot interleave because it takes start_lock, which start() holds for the whole sequence. a_run_that_ends_immediately_leaves_no_entry_behind pre-signals shutdown so the loop breaks on its first check - the shortest path from spawn to remove_if - and runs many rounds on a multi-threaded runtime. It probes the registry directly rather than through state(), which answers None both for "no entry" and for "the state lock was momentarily held" and would otherwise read a transient lock as a clean registry. Hitting the race is probabilistic; the invariant is not. With the handle registered before the spawn there is no ordering in which a finished loop leaves an entry behind, so the test cannot fail spuriously. Pre-existing upstream bug, carried in this PR because it lives in the function the loop-engineering change rewrites. * test(goals): assert on the registry's own agent name AgentEntry carries both `name` (what the registry holds) and `manifest.name` (what the caller asked for). The two diverge when a spawn path renames for uniqueness, and the property under test is what the registry actually ended up holding, so `name` is the field that answers it. `list_arcs` sorts on the same field. * docs(changelog): point the fragment at this PR's number The trailing (#N) group is what the release flow matches against the generated line for this PR, so it has to be #7785. #6505 stays as a mid-bullet cross-reference, which the tooling ignores by design. * fix(dashboard): remove unnecessary escape in goals test * fix(goals): restore goal_run_start signature lost during rebase * fix(goals): sync start_goal_run call with upstream 7-arg signature Move the kernel call after parameter extraction so loop-engineering fields (verify_agent_id, verify_max_retries, evaluator_model) are resolved before they are passed. Update the test call site to match. * fix(goals): resolve all rebase artifacts in loopeng branch Restore kernel_api.rs from origin/main and apply the loop engineering parameter additions on top — the prior rebase replaced the entire impl block with self-recursive default methods, producing 133 compilation errors. Move loop-engineering parameter extraction in routes/goals.rs before the start_goal_run call so the new 7-arg signature is satisfied. * fix(goals): add missing args to goal_runner test call site The start() method takes 11 args after the loop-engineering addition. The handle-unset test was still passing only 5. * fix(goals): update test call sites to match 11-arg start() signature Two tests still used the old 5-arg start() signature after the verifier gate was added. Update both call sites to pass the six new arguments (on_learnings_captured, evaluate_goal, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model) and remove the duplicate test that was added during rebase. * fix(goals): send GOAL_LEARNED lessons to the pending approval queue A goal run wrote its captured lessons straight into the installed skills directory, gated only by the goal's `loop_engineering` flag. That was a second path to skill creation that approved itself, standing next to a workshop (#3328) that deliberately requires an explicit `librefang skill pending approve` before any machine-proposed skill reaches an agent's prompt — and it was reachable by an agent emitting one marker line in its own output. The prompt-injection scan did run, but that is only half the boundary; the other half is that a human has read the thing. The lessons now become a `CandidateSkill` in `pending/<agent>/`, the same queue the background skill reviewer already files into, and only approval installs them. A second run of the same goal files a `CandidateKind::Update` draft when the first run's skill is already installed, so approval routes through `update_skill` rather than failing on the name already existing and dropping what the second run learned. Cap and TTL come from the producing agent's `[skill_workshop]` block, read once at run start. Drafts that did not come from a conversation turn are tagged with a sentinel `explicit_instruction` trigger naming the producer, the convention the reviewer already uses for `auto_evolve_reviewer`; this one is `goal_learned`. Nothing about the durable record changes: the runner still writes `goal_learnings_<id>` to the shared store before any of this, so a draft that is capped out or rejected costs the agent a convenience, not the lessons. * docs(goals): say what an unresolvable evaluator_model does, and give the tick breaker its own entry `verify_agent_id` is validated at save time and `evaluator_model` is not, which reads as an oversight until the asymmetry is written down. A verifier id has a checkable shape and no correct value that fails the check; a model id has neither, and whether one resolves depends on the provider configuration at the moment of the call rather than at save time. So a save-time 400 would reject a model the operator is about to configure and would still not guarantee the id resolves when the run reaches it. The behaviour that is already in place is the right one — a `WARN` per iteration and a fall back to the agent's own marker — and it is now stated on the field and next to the route that stores it. The five-consecutive-failure circuit breaker moves out of the loop-engineering fragment's last line into its own entry, because it is a fix an operator recognises on its own terms: a run pointed at a deleted agent now ends in `Stopped` carrying the error instead of in `MaxIterationsReached` carrying nothing. * fix(kernel,api): close the loop-engineering verifier gate's back doors The #7785 review found that a goal under loop_engineering could still finish on work its own verifier had just rejected, through routes the gate did not close: rejected progress crossed the completion boundary through the pre-existing progress>=100 check, an unreachable verifier fed no circuit breaker and burned the whole iteration budget, and GoalRunner::start's own refusal was discarded and reported as success. Also closes: the completion judge grading a blank statement for a title-only goal, a second run of a goal overwriting the first run's captured lessons, and verify_max_retries/verify_agent_id skipping the boundary validation their sibling fields already have. * fix(kernel,api): close the goal_update tool's verifier-gate bypass /review of 82643bf found the progress clamp protected only one of progress's two writers. goal_update is a first-class tool the agent's own system prompt tells it to call, and it patches the same shared document the runner reads independently of parse_tick, so a rejected iteration could still close the run one tick later through the top-of-loop progress>=100 check. The real fix is in that reader: treat bare progress as completion only when no verifier is configured to bypass, which also lets the clamp stop reaching the plain no-verifier and accepted-but-undeclared-done paths it was pinning at 99 for no reason. Also: move the dead-verifier Stopped break after the same per-iteration bookkeeping every other outcome gets, so the run API stops undercounting iterations already paid for; gate the GOAL_BLOCKED exit by the same verified flag GOAL_DONE already uses, closing the one remaining route by which a rejected iteration could end a run; correct a stale comment and an internal-error response that both claimed a 409 which does not exist in this file (the real condition is the goal being gone, i.e. 404); restore a doc comment two of the new #6562 tests had eaten the antecedent of; and fix a changelog fragment whose PR attribution had drifted onto its own line. * fix(kernel): gate the goal_update tool's status field the same as progress Re-review of 1a627f1 found the same tool has a second field that bypassed the verifier gate: goal_update's status="completed" hit the identical top-of-loop GoalStatus::Completed check with no verifier involved, since the field is a fresh enum write untouched by parse_tick. Same fix as progress: bare Completed only ends a verified run through the runner's own done branch now; Cancelled stays unconditioned since it is a legitimate external stop order, not something the gate itself produces. Also: revert the blocked-marker gate from the previous commit — GOAL_BLOCKED is a claim about the agent's own situation, not the task the verifier grades, and gating it left a genuinely blocked agent unable to stop a verified run before burning its whole iteration budget for no completion-safety benefit (blocked never reaches Completed). Rewrite the changelog fragment that still described the superseded write-side clamp instead of the reader-side gate that actually ships. Correct a test doc comment that overclaimed which function the tool's write path shares with the simulation. Pause the clock in three new tests that were sleeping through real TICK_INTERVAL delays. Restore a doc comment line broken across three lines by the previous reflow. * test(kernel): make the status-bypass regression test actually discriminate The prior version passed identically against the commit it was meant to guard against status="completed" bypassing the verifier gate: a successful tick always overwrites goal.status back to InProgress at the end of the same iteration (new_status is never None on that path), so the tool-written Completed never survived to the next header check regardless of whether the header gated on it. The real window is a turn that calls the tool and then fails, since the Err arm never touches the goal document at all. The first tick now fails after writing, which makes the test red against the parent commit's header check and green against this one — confirmed by hand-reverting the header condition, running the test, and reverting back. * fix(goals): give the operator a real stop, and a breaker to every gate leg Six findings from the #7785 re-review. The verifier gate landed in the reader, which closed the `goal_update` tool's bypass but also took the operator's completion path with it — a `PUT` marking a gated goal `completed` no longer stopped the run, and the iteration in flight wrote it back to `in_progress`. Rather than guess which writer produced a stored value, route the operator through the run's real control channel: `update_goal_by_id` now calls `stop_goal_run` the way `delete_goal` always has, and the runner stops writing its own status and progress over a goal whose run has been stopped. The shared top-of-loop condition is unchanged, so #7973 stays converged. The verifier's dispatch errors never reached `classify_tick_error`, so a throttled verifier was reported as a dead one and `RateLimited` never fired for that leg; the rework turn had no breaker at all and could run to the iteration cap. Each leg now carries its own `GateStreak`, split by kind — sharing one counter between legs, or reusing the generator's `error_streak`, produces a counter that is reset or never read. Learnings were appended before the gate and again per rework round with no dedup, so a restated lesson was stored once per round; they now follow the same replace-the-rejected-reply rule as `parsed`, and the run refuses a text it already holds. Both goal endpoints now reject `verify_agent_id == agent_id`, update comparing the effective post-update pair, and the dashboard picker no longer offers the assigned agent. Create drops a blank `evaluator_model` instead of storing "", which is what an earlier review reply claimed it already did. Every test was run against the reverted production block first and recorded failing. * test(api): cover the operator stop at the injection site, not just the helper The kernel test covers the runner's half of the interlock; nothing covered the wiring itself, so deleting `stop_goal_run` from `update_goal_by_id` left the suite green. Four `TestServer` tests: start a run, `PUT` the terminal status, then read back. The follow-up `POST /stop` asserting `stopped: false` is the mechanism-precise part — the registry entry is removed synchronously during the `PUT`, so a second stop finds nothing left. Covers `cancelled` as well as `completed`, and the no-verifier path alongside the gated one that the review reported. The fourth is the negative: an ordinary edit must leave the run alone, which is what stops the interlock from being wired unconditionally. * fix(goals): let a bare operator stop keep the iteration's accounting goal_runner.rs read a plain AtomicBool and skipped patch_goal on any stop, so the two operator paths were treated as one when they are opposites. A terminal PUT /api/goals/{id} writes the document before stopping, and the runner's own end-of-iteration write is exactly what would revert it: new_status is InProgress for every iteration that did not pass verification, and the PUT carries progress too. That write has to be barred. A POST /api/goals/{id}/stop writes nothing. There is no operator choice on the document to protect, so skipping the write only throws away accounting already paid for: the goal keeps the previous iteration's progress while the run row reports the higher iteration count those turns were billed at. Two numbers that contradict each other, with nothing telling the operator which one counts. StopFlag carries who raised it, and stop_after_goal_write is a separate entry point taken only by the terminal PUT. Deletion keeps the plain stop: the rows are already gone, so an in-flight patch_goal updates nothing. wrote_goal is stored before stopped so a thread observing the stop also observes ownership; the reverse order leaves the exact window this distinction exists to close. Verified in red by restoring the previous semantics: the bare-stop test fails with progress 0 against the expected 60, and the terminal-PUT direction alone would have passed either way. --------- Co-authored-by: Evan <suzukaze.haduki@gmail.com>

librefang#7785 landed on main as a squash, which severs the ancestry between the commits on this branch and the identical content now on main, so git saw six files edited on both sides and refused to pick. Five of them resolve to this branch's version, verified first rather than assumed: for `goals.rs`, `goals_routes_integration.rs`, `goal_runner.rs`, `goal_lifecycle.rs` and `goal.rs`, main's side is byte-identical to librefang#7785's head and this branch's side is byte-identical to this branch's head, so taking ours keeps librefang#7785's content and this branch's additions on top of it. `kernel_api.rs` is the exception and was merged by hand. Main's copy is *not* librefang#7785's, because librefang#8271 has since changed `reconnect_mcp_server` to return `crate::McpReconnectError` instead of `String`. Taking ours there would have quietly reverted a fix that is already on main. The resolved file is this branch's version with librefang#8271's two hunks reapplied; it differs from this branch's head only in those hunks, and from main only by 42 insertions and no deletions.

Upstream squash-merged librefang#7973 while the integration already carried its branch, so `goal_run_start_or_resume` ended up with two copies of the `require_paused && paused_run.is_none()` block: main's, before the goal is looked up, and the branch's, after it. Git raised no conflict — the two sit at different points in the same function — and the first one wins, which makes the second dead code. The order is the whole point. `require_paused` is a precondition on a goal that *exists*; checking it first answers a well-formed but unknown id with `409` and the advice "use POST /api/goals/{id}/start instead" — a start that would itself 404, because `/start` is this same function with `require_paused: false` and it does reach the lookup. The branch had already fixed that, with the comment explaining it, and the merge quietly reinstated the behaviour in front of the fix. `goal_run_resume_on_an_unknown_goal_is_a_404_not_a_conflict` was red against the merged tree with `left: 409, right: 404`, which is exactly this. The stale copy is gone, its `run_state` / `paused_run` bindings with it; the surviving block keeps its rationale. `cargo nextest run -p librefang-api --test goals_routes_integration`: 82 passed, 0 failed.

Merging `origin/main` after librefang#7973 landed there as a squash put a second copy of the `require_paused && paused_run.is_none()` block into `goal_run_start_or_resume`: main's, before the goal is looked up, and this branch's, after it. Git raised no conflict — the two sit at different points in the same function — and the first one wins, so the branch's fix became dead code. The order is the point. `require_paused` is a precondition on a goal that *exists*; checking it first answers a well-formed but unknown id with `409` and the advice "use POST /api/goals/{id}/start instead" — a start that would itself 404, because `/start` is this same function with `require_paused: false` and it does reach the lookup. `goal_run_resume_on_an_unknown_goal_is_a_404_not_a_conflict` was red in CI on this branch with `left: 409, right: 404`, which is exactly this. The stale copy is gone, its `run_state` / `paused_run` bindings with it; the surviving block keeps its rationale and sits after the lookup.

…dout (#8359) * fix(kernel): restore checkpoint verify_max_retries on paused goal readout GoalRunner::state() reconstructs a paused run from its checkpoint once the loop task has exited, and the checkpoint carries verify_max_retries for exactly this reason: it is the one loop-engineering value the goal document never holds, since it is a per-run number the operator sets on the start body rather than part of the goal's own configuration. Reading it from the goal document instead meant a run started with {"verify_max_retries": 8} reported the compiled default once paused, and the bodyless /resume that follows a readout resolves its own budget the same way state() does, so a paused goal's operator-set retry budget silently disappeared on resume with nothing failing. This fix and its two regression tests shipped once already on the branch that introduced pause/resume for loop-engineered goals, but #7973 was merged into main by squash while that branch was still live, and the squashed commit that landed did not carry them. * docs(changelog): rename fragment to match the actual PR number The fragment was written before opening the pull request; GitHub assigned #8359, not the #8358 anticipated.

* feat(dashboard): add pause/resume controls to the goals page Wire up the pause and resume goal mutations in GoalsPage so users can pause a running goal and resume a paused one. Adds "paused" state badge, Pause/Resume buttons, and i18n keys. * fix(dashboard): add goal pause/resume i18n keys to ko/pl and fix test mocks * feat(goals): add pause/resume and a configurable loop cadence - Add `Paused` variant to `GoalRunPhase` so a suspended run preserves its iteration count and progress for later resumption. - `tick_interval_secs` on the goal document lets an operator tune the delay between loop iterations (default 2s, clamped 1–86400). - POST /api/goals/{id}/pause — cooperative pause; the in-flight turn finishes, then checkpoints and exits. - POST /api/goals/{id}/resume — continues from the checkpoint; refuses with 409 when there is nothing to resume. - Pause checkpoints are stored in the shared KV store and cleared on resume and on stop, so a stale checkpoint never seeds a fresh start. - Integration tests cover pause-signals-a-live-run, idle-goal-reports- false, and resume-without-pause-is-conflict. Refs #5744 * fix(goals): pass substrate to GoalRunner in tests * fix(dashboard): goal pause/resume controls track the run-level phase and drop dead fallbacks Address #8029 review feedback: - Drop the goal-status "paused" arms from progressForGoalStatus and goalStatusBadgeVariant: pause is a run-level phase (GoalRunPhase::Paused from #7973), and GoalStatus never emits "paused", so those arms were unreachable and modelled the wrong concept. - Remove the dead defaultValue fallbacks from run_pause, run_resume and run_stop — all three keys exist and are translated in every locale, and a present default hides a future missing-key regression. - Add a translated run_phase_paused label to all five locales and a paused arm to the run-phase pill, which the stacked backend makes reachable. - Cover the pause and resume buttons with tests asserting the mutations fire with the goal id. - Add the changelog fragment. The branch is stacked on #7973, which supplies POST /api/goals/{id}/pause and /resume and GoalRunPhase::Paused. * fix(goals): reject a pause checkpoint whose max_iterations is missing load_pause_checkpoint parsed agent_id with `?` but read max_iterations with unwrap_or(0), answering the same corrupt-checkpoint question two different ways. A cap of zero is not a state an operator can reach: the API rejects max_iterations: 0 as a bad request and goal_run_start clamps it up with .max(1), so a partial checkpoint surfaced through GoalRunner::state as a paused run reporting a cap nobody chose. Treat it like agent_id and fall through to None, which puts a damaged checkpoint on the restart path an unreadable one already takes. iteration and last_progress keep unwrap_or(0) because zero is a real value for both. * fix(goals): resume a paused run under its own cap and timestamps `POST /api/goals/{id}/resume` dispatched with no body, so the kernel substituted `DEFAULT_GOAL_MAX_ITERATIONS` for the cap the paused run was actually under. `GoalRunner::start` seeded the iteration count from the checkpoint while taking the cap from its argument, so a run started with 100 and paused at iteration 30 resumed under a cap of 25 — past the loop's own guard before its first turn, and the exit that follows clears the checkpoint, so the progress the resume was asked to continue was unrecoverable. The cap is now resolved in `GoalRunner::start`, the only layer that knows whether this is a resume: an explicit value wins, then the checkpoint's, then the compiled default. `/resume` accordingly accepts the same optional body `/start` does, and an explicit `max_iterations` there is a deliberate re-budgeting of the remaining run rather than something to ignore. A paused run's `paused_at` was written to the checkpoint and never read back, and `state()` stamped both timestamps with the clock, so two consecutive `GET /api/goals/{id}/run` on a motionless goal disagreed and "how long has this been paused" computed to roughly zero. `ResumePoint` now carries `started_at` and `paused_at`, the checkpoint stores the run's real start time, and both `state()` and `start()` restore them: a resume continues the run it checkpointed rather than opening a new segment, so it keeps the moment it actually began. A checkpoint predating these fields still resumes, falling back to the clock and the default cap. Also drops `goal_tick_sender_context`'s `display_name` parameter, which every caller filled with the same constant the function already hardcodes into `channel`. Tests: five `#[tokio::test]` cases against the real router drive a resume through `POST /resume` from a seeded checkpoint and pin the cap, the override, its validation, and both timestamps; three kernel tests pin the checkpoint round-trip and the cap precedence at the layer that decides it. * feat(goals): gate autonomous runs on a verifier and an evaluator A goal run ended the moment the agent wrote GOAL_DONE, which made the worker the sole judge of its own work - the one check a long-horizon loop most needs, and the one it did not have. Setting loop_engineering on a goal adds two judges that are not the worker. A verifier agent reads each iteration's output and returns VERDICT: PASS|FAIL|NEEDS_REWORK; a rejection goes back to the generator carrying the verifier's stated reason, up to verify_max_retries rework rounds, and until the verifier passes the work GOAL_DONE does not end the run. An evaluator model makes one cheap yes/no read of the goal against the latest output and can conclude the goal is met even when the agent never claimed it. The agent can also record a reusable lesson with GOAL_LEARNED: <text>. Captured lessons are replayed into later iterations' prompts, persisted under goal_learnings_<id>, and written into a goal-learned-* skill through the same prompt-injection scan every other skill-creation path uses. All three are inert unless the goal opts in: with loop_engineering off the prompt is byte-identical to the one it was and the run still makes exactly one LLM call per iteration. a_plain_run_makes_no_extra_llm_calls asserts that, because an evaluator that fires on every goal would double every existing operator's bill silently. Sub-agents are delegated, not provisioned. The prompt directs the agent to its own agent_spawn / agent_send tools, which run under the capabilities its operator granted; neither the runner nor the API ever creates an agent on a caller's behalf. loop_engineering_without_a_verifier_provisions_no_agent and goal_run_start_with_an_unusable_agent_id_provisions_no_agent assert the registry does not grow, which is the property that survives a later refactor - asserting only the status code would not. An unusable verify_agent_id is a 400 with the id named rather than a silently ignored field, because dropping it downgrades a gated run to an ungated one without telling the operator who configured the gate. A run now also gives up after five consecutive non-rate-limit tick failures instead of spending its whole iteration budget rediscovering a deleted agent or a revoked key. Refs #6505 * fix(goals): register a goal run's handle before spawning its loop start() spawned the loop task and inserted its RunHandle afterwards. A loop that finishes inside that window runs its self-cleanup remove_if against a registry that does not hold it yet: the removal finds nothing, the insert then lands a handle for a run that is already over, and nothing ever collects it. state() reports the run as present forever and the registry grows by one every time it happens. The window is short, but the exits that fit inside it are the fast ones - a pre-signalled shutdown, or a goal deleted between the API's read and this call - both of which end the loop before its first agent turn. The handle is now registered before the spawn and the JoinHandle backfilled after. A missing entry at backfill time means the loop already finished and cleaned up, so dropping the handle there is correct: there is nothing left to abort. The generation check keeps a replacement run's handle from being overwritten, and stop() cannot interleave because it takes start_lock, which start() holds for the whole sequence. a_run_that_ends_immediately_leaves_no_entry_behind pre-signals shutdown so the loop breaks on its first check - the shortest path from spawn to remove_if - and runs many rounds on a multi-threaded runtime. It probes the registry directly rather than through state(), which answers None both for "no entry" and for "the state lock was momentarily held" and would otherwise read a transient lock as a clean registry. Hitting the race is probabilistic; the invariant is not. With the handle registered before the spawn there is no ordering in which a finished loop leaves an entry behind, so the test cannot fail spuriously. Pre-existing upstream bug, carried in this PR because it lives in the function the loop-engineering change rewrites. * test(goals): assert on the registry's own agent name AgentEntry carries both `name` (what the registry holds) and `manifest.name` (what the caller asked for). The two diverge when a spawn path renames for uniqueness, and the property under test is what the registry actually ended up holding, so `name` is the field that answers it. `list_arcs` sorts on the same field. * docs(changelog): point the fragment at this PR's number The trailing (#N) group is what the release flow matches against the generated line for this PR, so it has to be #7785. #6505 stays as a mid-bullet cross-reference, which the tooling ignores by design. * fix(dashboard): remove unnecessary escape in goals test * fix(goals): restore goal_run_start signature lost during rebase * fix(goals): sync start_goal_run call with upstream 7-arg signature Move the kernel call after parameter extraction so loop-engineering fields (verify_agent_id, verify_max_retries, evaluator_model) are resolved before they are passed. Update the test call site to match. * fix(goals): resolve all rebase artifacts in loopeng branch Restore kernel_api.rs from origin/main and apply the loop engineering parameter additions on top — the prior rebase replaced the entire impl block with self-recursive default methods, producing 133 compilation errors. Move loop-engineering parameter extraction in routes/goals.rs before the start_goal_run call so the new 7-arg signature is satisfied. * fix(goals): add missing args to goal_runner test call site The start() method takes 11 args after the loop-engineering addition. The handle-unset test was still passing only 5. * fix(goals): update test call sites to match 11-arg start() signature Two tests still used the old 5-arg start() signature after the verifier gate was added. Update both call sites to pass the six new arguments (on_learnings_captured, evaluate_goal, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model) and remove the duplicate test that was added during rebase. * fix(goals): send GOAL_LEARNED lessons to the pending approval queue A goal run wrote its captured lessons straight into the installed skills directory, gated only by the goal's `loop_engineering` flag. That was a second path to skill creation that approved itself, standing next to a workshop (#3328) that deliberately requires an explicit `librefang skill pending approve` before any machine-proposed skill reaches an agent's prompt — and it was reachable by an agent emitting one marker line in its own output. The prompt-injection scan did run, but that is only half the boundary; the other half is that a human has read the thing. The lessons now become a `CandidateSkill` in `pending/<agent>/`, the same queue the background skill reviewer already files into, and only approval installs them. A second run of the same goal files a `CandidateKind::Update` draft when the first run's skill is already installed, so approval routes through `update_skill` rather than failing on the name already existing and dropping what the second run learned. Cap and TTL come from the producing agent's `[skill_workshop]` block, read once at run start. Drafts that did not come from a conversation turn are tagged with a sentinel `explicit_instruction` trigger naming the producer, the convention the reviewer already uses for `auto_evolve_reviewer`; this one is `goal_learned`. Nothing about the durable record changes: the runner still writes `goal_learnings_<id>` to the shared store before any of this, so a draft that is capped out or rejected costs the agent a convenience, not the lessons. * docs(goals): say what an unresolvable evaluator_model does, and give the tick breaker its own entry `verify_agent_id` is validated at save time and `evaluator_model` is not, which reads as an oversight until the asymmetry is written down. A verifier id has a checkable shape and no correct value that fails the check; a model id has neither, and whether one resolves depends on the provider configuration at the moment of the call rather than at save time. So a save-time 400 would reject a model the operator is about to configure and would still not guarantee the id resolves when the run reaches it. The behaviour that is already in place is the right one — a `WARN` per iteration and a fall back to the agent's own marker — and it is now stated on the field and next to the route that stores it. The five-consecutive-failure circuit breaker moves out of the loop-engineering fragment's last line into its own entry, because it is a fix an operator recognises on its own terms: a run pointed at a deleted agent now ends in `Stopped` carrying the error instead of in `MaxIterationsReached` carrying nothing. * fix(dashboard): the run-phase union and the badge must know about "paused" GoalRunState.phase is "typed from the wire contract rather than string so a caller cannot pass a typo'd literal" — but this PR's own server now sends "paused", a value the type said could not exist. The badge then fell through to the default styling and GoalsPage rendered a bare untranslated "paused" in every language, while every other phase had a locale key. The union gains "paused", the badge gains a case (warning palette — the run is alive but not progressing, same family as stopped), and goals.run_phase_paused lands in all five locales. The pause/resume dashboard client (pauseGoalRun / resumeGoalRun wrappers and their mutation hooks) is deliberately not here: it arrives in #8029, which is stacked on this branch and adds the controls that use it. * fix(kernel): goal runner resume, verifier gate, and pause correctness Four defects the maintainer review of #7973 found unaddressed after the review pass: - run_loop counted iterations from a hardcoded 0 instead of the checkpointed value start() already resolved into the run's state, so a resumed run got a fresh iteration budget rather than continuing the one it was paused under. - A verifier-rejected iteration's unclamped GOAL_PROGRESS: 100 let the next iteration's plain progress>=100 check finish the run anyway, bypassing the verifier gate entirely. - Pause/stop were only observed at the top of the outer loop, so a tick_interval_secs near its 24h ceiling left a pause request unobserved for up to a day; the inter-tick sleep now polls every second instead of sleeping the whole interval in one shot. - Pausing discarded whatever GOAL_LEARNED: lessons the run had captured so far. The resume checkpoint now carries them and threads them back into the resumed run's own accumulator. * fix(kernel): goal run learnings respect the skill workshop opt-in queue_learnings_as_pending_skill was called unconditionally from the goal runner's on_learnings hook, so a goal run queued a pending skill draft even for agents that never set [skill_workshop] enabled = true in agent.toml. Nothing else in the goal-run path consulted the workshop config -- only the approval-side CLI/API/dashboard did. Extracted the gate into should_queue_learnings (enabled && auto_capture), matching the check every other automatic capture path in the workshop already applies. * fix(api): validate verify_max_retries and blank verifier/evaluator fields start_or_resume read verify_max_retries with a bare .map(|n| n as u32), so an out-of-range or wrongly-typed value silently wrapped instead of 400ing -- the same field max_iterations already validates properly a few lines above. create_goal's verify_agent_id parsing didn't go through optional_uuid_field, so a blank string (the create form's clear signal, per #6562) 400ed as "Invalid verify_agent_id" instead of being treated as unset. evaluator_model_str had no blank filter either, so a blank string was stored verbatim as Some(""), contrary to the field's own documented None-means-unconfigured contract. * fix(cli): goal --watch recognizes the paused run phase terminal_phase_message had no branch for "paused", so a goal that transitioned into paused mid-watch (an operator pausing it from the dashboard, say) read as an unclassified state: classify_poll routed it to Unobservable, burning the bounded retry budget before --watch gave up with a generic "outcome unknown" message instead of reporting that the run was paused. Added the branch plus a locale key in all four CLI locales, and extended every_terminal_phase_has_a_summary to cover it. * fix(kernel): close the goal_update verifier-gate bypass and harden pause/resume /review of 56e75aee0..562f414b5 found the progress clamp from the first pass protected only one of goal.progress's two writers: goal_update (the tool the agent's own system prompt tells it to call) patches the same shared document directly, bypassing parse_tick entirely, so a rejected iteration could still close the run one tick later through the bare top-of-loop progress>=100 / status==Completed check. PR #7785 found and fixed the identical bug first; adopted its reader-side gate verbatim (bare progress/status only count as done when no verifier is configured; Cancelled stays unconditioned) so the two PRs converge instead of one reverting the other on merge, and kept the clamp as defense in depth scoped to !verified. Also from the same review: - The inter-tick sleep now wakes to a fixed deadline via sleep_until instead of chaining PAUSE_POLL_INTERVAL sleeps end-to-end, which would have drifted by scheduling latency accumulated once per slice (on the order of a minute over the 24h maximum tick interval). - verify_max_retries now survives a pause the same way max_iterations already does: added to the resume checkpoint with the same argument-then-checkpoint-then-default precedence. - run_loop_resumes_from_the_checkpointed_iteration_not_zero passed identically with and without the iteration-reset fix it was meant to guard, because the final iteration count at MaxIterationsReached is the same either way. Now counts turns and inspects the first resumed prompt, which does discriminate. - learnings_survive_a_pause_and_resume_cycle asserted an exact ordered list that only held if exactly one tick landed before pause() took effect; a second tick under load duplicated a fixed lesson. The send closure now returns a distinct lesson per turn and the assertion checks membership/uniqueness instead of an exact list. - Added a regression test for pause and shutdown signalled together mid-sleep: both now resolve through the same pause-before-shutdown check at the top of the loop, so they land on Paused rather than the old flat select! hardcoding Stopped in that one path. * fix(kernel): move the skill-workshop opt-in gate into its single callee /review of 83e9bfecf found should_queue_learnings gated only the caller (goal_run_start's on_learnings closure), leaving queue_learnings_as_pending_skill's own two pre-existing tests untested for the opt-in -- both called it directly with the disabled default and asserted a draft got queued, which is exactly what the fix was meant to stop. Moved the gate inside the function itself (its only production caller), updated those two tests to enabled: true since they were asserting the pre-fix behavior, and added a test asserting a disabled workshop queues nothing when called directly. * fix(api): refuse a resume cap at or below the checkpoint's iteration /review of 56e75aee0 found a side effect of the iteration-reset fix: GoalRunner::start now compares an explicit max_iterations against the RESTORED iteration count instead of 0, so a resume body naming a cap at or below that count (e.g. {"max_iterations": 10} on a run paused at iteration 30) trips the cap check on the very first pass with no turn run, and clears the checkpoint -- and the learnings it carries -- on the way out, for a request that could never have advanced the run. start_or_resume now reads the goal's run state once (reused for the existing require_paused check) and rejects such a cap with a 400 naming both numbers, on both /start and /resume since /start also auto-resumes from an existing checkpoint. * fix(kernel): correct stale doc comments and cover the registry-absent case /review of 378b8bfa4 found two loose ends: - The doc comments on GoalRunner::start and its test described an explicit resume max_iterations as "re-budgeting the remaining run", which is not what the code does or what the earlier fix in this branch made it do: the cap is a TOTAL ceiling compared against the restored iteration count, exactly like a fresh start compares against 0. Corrected both, and noted the new below-checkpoint 400 in the API route's own doc comment. - should_queue_learnings' own tests only exercised it as a pure predicate; nothing verified the other half of goal_run_start's wiring, that an agent absent from the registry (deleted, never spawned, mistyped id) still resolves to a denying config rather than some other default. Extracted that resolution into resolve_workshop_config so it's callable without a live turn, and added two tests: absent-agent denies, and a registered agent's own manifest setting is what actually gets read (proving the first test isn't passing because the function ignores the registry outright). Verified the new test actually guards something: temporarily made the fallback permissive (enabled: true instead of ::default()) and confirmed resolve_workshop_config_denies_when_the_agent_is_absent_from_the_registry fails before restoring it. * fix(api): resume answers 404 for a goal that does not exist The `require_paused` precondition ran between `parse_goal_id` and the substrate read that resolves the goal, so `POST /api/goals/{id}/resume` answered a well-formed but unknown id with 409 and the advice to use `POST /api/goals/{id}/start` instead — the same handler body with `require_paused: false`, which reaches the lookup and would itself have answered 404. An operator who mistyped an id was told the wrong thing and pointed at a request that could not work. Move the run-state read and the precondition below the goal lookup. The 409 is unchanged for its real case, a goal that exists with nothing paused, and it still precedes the agent_id validation so no other status moves. `/stop`, `/pause` and `/run` are deliberately not part of this: none of them looks the goal up, and all three answer 200 with a false flag for an unknown id. * fix(kernel): cancel discards a pause checkpoint it cannot read `load_pause_checkpoint` answers `None` for two different things: there is no checkpoint, and there is a row it could not turn into a `ResumePoint`. A substrate read error takes the second path through its `.ok().flatten()?`, and so does a row whose `agent_id` is missing or not a UUID. `stop_locked` gated the delete on that `None`, so in either case it skipped the delete and left the row behind — and the next `/start` auto-resumes from a checkpoint, so a transient storage failure at cancel time silently resurrected the run the operator had just cancelled. That is the outcome the comment directly above the delete says cancel exists to prevent. Delete unconditionally and use the read only for the return value, which still reports whether a resume point was readable. Costs nothing: `clear_pause_checkpoint` was already a no-op on a missing key. The test asserts the raw key rather than `load_pause_checkpoint`, which is `None` before and after the delete on this input and so cannot tell the fix from the bug. * test(dashboard): move the unknown-phase example off "paused" #8067 used `paused` as its stand-in for a phase the daemon emits before the dashboard knows it, which is exactly the gap this PR closes. With `paused` now carrying its own switch arm the example had to become a phase the switch still does not know, and `paused` joins the API-emittable list with a test asserting its own variant rather than the neutral fallback. * fix(goals): restore the resume path's loop-engineering arguments Rebasing this branch onto `origin/main` flattens the twenty-five merge commits it accumulated, and a flattened replay resolves each commit against its own parent instead of against the resolution the merge already chose. Three of those resolutions did not survive the replay, and the first one does not compile. - `KernelApi::resume_goal_run` and `LibreFangKernel::goal_run_resume` came back with their original three arguments while `routes/goals.rs` calls them with seven. A resume that does not carry `loop_engineering` / `verify_agent_id` / `verify_max_retries` / `evaluator_model` silently drops the verifier gate the operator configured on the goal, which is the failure the gate exists to prevent. - `GoalRunner::start` lost the block that reconstructs those four values from the goal document when a run is resumed, so a resumed run reported no verifier even when the goal has one. - `ko.json` ended up with `goals.run_phase_paused` twice. Both spellings parse, the later one wins, and nothing warns. Each file is restored to the three-way merge of this branch's own tip against `origin/main`, so what lands is the resolution the branch already made rather than a fresh guess at it. * fix(kernel): a paused run reported the default verifier retry budget `GoalRunner::state` reconstructs a paused run from its checkpoint once the loop task has exited and self-cleaned its registry slot, and it restored every field of that run from the checkpoint except the verifier retry budget, which it took from the compiled default instead. A run started with `{"verify_max_retries": 8}` therefore answered `GET /api/goals/{id}/run` with 3 while it was paused, and the bodyless `/resume` that follows went back to 8 — `GoalRunner::start` already resolves the checkpoint's value ahead of the default. The readout and the resume disagreed about the budget the run was under, and the resume was the one telling the truth. `ResumePoint::verify_max_retries` names this exact surface as the reason the field is carried at all, and the sibling `max_iterations` two fields up already reads the checkpoint, so this restores the field to the behaviour its own documentation describes rather than introducing a new rule. The `loop_engineering` gate is unchanged: a goal with the flag off still reports no budget, because reporting one would advertise a gate the resume will not apply. Both directions are covered — the restore, and the gate that must keep answering zero without loop engineering. * feat(goals): gate autonomous runs on a verifier and an evaluator A goal run ended the moment the agent wrote GOAL_DONE, which made the worker the sole judge of its own work - the one check a long-horizon loop most needs, and the one it did not have. Setting loop_engineering on a goal adds two judges that are not the worker. A verifier agent reads each iteration's output and returns VERDICT: PASS|FAIL|NEEDS_REWORK; a rejection goes back to the generator carrying the verifier's stated reason, up to verify_max_retries rework rounds, and until the verifier passes the work GOAL_DONE does not end the run. An evaluator model makes one cheap yes/no read of the goal against the latest output and can conclude the goal is met even when the agent never claimed it. The agent can also record a reusable lesson with GOAL_LEARNED: <text>. Captured lessons are replayed into later iterations' prompts, persisted under goal_learnings_<id>, and written into a goal-learned-* skill through the same prompt-injection scan every other skill-creation path uses. All three are inert unless the goal opts in: with loop_engineering off the prompt is byte-identical to the one it was and the run still makes exactly one LLM call per iteration. a_plain_run_makes_no_extra_llm_calls asserts that, because an evaluator that fires on every goal would double every existing operator's bill silently. Sub-agents are delegated, not provisioned. The prompt directs the agent to its own agent_spawn / agent_send tools, which run under the capabilities its operator granted; neither the runner nor the API ever creates an agent on a caller's behalf. loop_engineering_without_a_verifier_provisions_no_agent and goal_run_start_with_an_unusable_agent_id_provisions_no_agent assert the registry does not grow, which is the property that survives a later refactor - asserting only the status code would not. An unusable verify_agent_id is a 400 with the id named rather than a silently ignored field, because dropping it downgrades a gated run to an ungated one without telling the operator who configured the gate. A run now also gives up after five consecutive non-rate-limit tick failures instead of spending its whole iteration budget rediscovering a deleted agent or a revoked key. Refs #6505 * fix(goals): register a goal run's handle before spawning its loop start() spawned the loop task and inserted its RunHandle afterwards. A loop that finishes inside that window runs its self-cleanup remove_if against a registry that does not hold it yet: the removal finds nothing, the insert then lands a handle for a run that is already over, and nothing ever collects it. state() reports the run as present forever and the registry grows by one every time it happens. The window is short, but the exits that fit inside it are the fast ones - a pre-signalled shutdown, or a goal deleted between the API's read and this call - both of which end the loop before its first agent turn. The handle is now registered before the spawn and the JoinHandle backfilled after. A missing entry at backfill time means the loop already finished and cleaned up, so dropping the handle there is correct: there is nothing left to abort. The generation check keeps a replacement run's handle from being overwritten, and stop() cannot interleave because it takes start_lock, which start() holds for the whole sequence. a_run_that_ends_immediately_leaves_no_entry_behind pre-signals shutdown so the loop breaks on its first check - the shortest path from spawn to remove_if - and runs many rounds on a multi-threaded runtime. It probes the registry directly rather than through state(), which answers None both for "no entry" and for "the state lock was momentarily held" and would otherwise read a transient lock as a clean registry. Hitting the race is probabilistic; the invariant is not. With the handle registered before the spawn there is no ordering in which a finished loop leaves an entry behind, so the test cannot fail spuriously. Pre-existing upstream bug, carried in this PR because it lives in the function the loop-engineering change rewrites. * test(goals): assert on the registry's own agent name AgentEntry carries both `name` (what the registry holds) and `manifest.name` (what the caller asked for). The two diverge when a spawn path renames for uniqueness, and the property under test is what the registry actually ended up holding, so `name` is the field that answers it. `list_arcs` sorts on the same field. * docs(changelog): point the fragment at this PR's number The trailing (#N) group is what the release flow matches against the generated line for this PR, so it has to be #7785. #6505 stays as a mid-bullet cross-reference, which the tooling ignores by design. * fix(dashboard): remove unnecessary escape in goals test * fix(goals): restore goal_run_start signature lost during rebase * fix(goals): sync start_goal_run call with upstream 7-arg signature Move the kernel call after parameter extraction so loop-engineering fields (verify_agent_id, verify_max_retries, evaluator_model) are resolved before they are passed. Update the test call site to match. * fix(goals): resolve all rebase artifacts in loopeng branch Restore kernel_api.rs from origin/main and apply the loop engineering parameter additions on top — the prior rebase replaced the entire impl block with self-recursive default methods, producing 133 compilation errors. Move loop-engineering parameter extraction in routes/goals.rs before the start_goal_run call so the new 7-arg signature is satisfied. * fix(goals): add missing args to goal_runner test call site The start() method takes 11 args after the loop-engineering addition. The handle-unset test was still passing only 5. * fix(goals): update test call sites to match 11-arg start() signature Two tests still used the old 5-arg start() signature after the verifier gate was added. Update both call sites to pass the six new arguments (on_learnings_captured, evaluate_goal, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model) and remove the duplicate test that was added during rebase. * fix(goals): send GOAL_LEARNED lessons to the pending approval queue A goal run wrote its captured lessons straight into the installed skills directory, gated only by the goal's `loop_engineering` flag. That was a second path to skill creation that approved itself, standing next to a workshop (#3328) that deliberately requires an explicit `librefang skill pending approve` before any machine-proposed skill reaches an agent's prompt — and it was reachable by an agent emitting one marker line in its own output. The prompt-injection scan did run, but that is only half the boundary; the other half is that a human has read the thing. The lessons now become a `CandidateSkill` in `pending/<agent>/`, the same queue the background skill reviewer already files into, and only approval installs them. A second run of the same goal files a `CandidateKind::Update` draft when the first run's skill is already installed, so approval routes through `update_skill` rather than failing on the name already existing and dropping what the second run learned. Cap and TTL come from the producing agent's `[skill_workshop]` block, read once at run start. Drafts that did not come from a conversation turn are tagged with a sentinel `explicit_instruction` trigger naming the producer, the convention the reviewer already uses for `auto_evolve_reviewer`; this one is `goal_learned`. Nothing about the durable record changes: the runner still writes `goal_learnings_<id>` to the shared store before any of this, so a draft that is capped out or rejected costs the agent a convenience, not the lessons. * docs(goals): say what an unresolvable evaluator_model does, and give the tick breaker its own entry `verify_agent_id` is validated at save time and `evaluator_model` is not, which reads as an oversight until the asymmetry is written down. A verifier id has a checkable shape and no correct value that fails the check; a model id has neither, and whether one resolves depends on the provider configuration at the moment of the call rather than at save time. So a save-time 400 would reject a model the operator is about to configure and would still not guarantee the id resolves when the run reaches it. The behaviour that is already in place is the right one — a `WARN` per iteration and a fall back to the agent's own marker — and it is now stated on the field and next to the route that stores it. The five-consecutive-failure circuit breaker moves out of the loop-engineering fragment's last line into its own entry, because it is a fix an operator recognises on its own terms: a run pointed at a deleted agent now ends in `Stopped` carrying the error instead of in `MaxIterationsReached` carrying nothing. * fix(kernel,api): close the loop-engineering verifier gate's back doors The #7785 review found that a goal under loop_engineering could still finish on work its own verifier had just rejected, through routes the gate did not close: rejected progress crossed the completion boundary through the pre-existing progress>=100 check, an unreachable verifier fed no circuit breaker and burned the whole iteration budget, and GoalRunner::start's own refusal was discarded and reported as success. Also closes: the completion judge grading a blank statement for a title-only goal, a second run of a goal overwriting the first run's captured lessons, and verify_max_retries/verify_agent_id skipping the boundary validation their sibling fields already have. * fix(kernel,api): close the goal_update tool's verifier-gate bypass /review of 82643bf7f found the progress clamp protected only one of progress's two writers. goal_update is a first-class tool the agent's own system prompt tells it to call, and it patches the same shared document the runner reads independently of parse_tick, so a rejected iteration could still close the run one tick later through the top-of-loop progress>=100 check. The real fix is in that reader: treat bare progress as completion only when no verifier is configured to bypass, which also lets the clamp stop reaching the plain no-verifier and accepted-but-undeclared-done paths it was pinning at 99 for no reason. Also: move the dead-verifier Stopped break after the same per-iteration bookkeeping every other outcome gets, so the run API stops undercounting iterations already paid for; gate the GOAL_BLOCKED exit by the same verified flag GOAL_DONE already uses, closing the one remaining route by which a rejected iteration could end a run; correct a stale comment and an internal-error response that both claimed a 409 which does not exist in this file (the real condition is the goal being gone, i.e. 404); restore a doc comment two of the new #6562 tests had eaten the antecedent of; and fix a changelog fragment whose PR attribution had drifted onto its own line. * fix(kernel): gate the goal_update tool's status field the same as progress Re-review of 1a627f16a found the same tool has a second field that bypassed the verifier gate: goal_update's status="completed" hit the identical top-of-loop GoalStatus::Completed check with no verifier involved, since the field is a fresh enum write untouched by parse_tick. Same fix as progress: bare Completed only ends a verified run through the runner's own done branch now; Cancelled stays unconditioned since it is a legitimate external stop order, not something the gate itself produces. Also: revert the blocked-marker gate from the previous commit — GOAL_BLOCKED is a claim about the agent's own situation, not the task the verifier grades, and gating it left a genuinely blocked agent unable to stop a verified run before burning its whole iteration budget for no completion-safety benefit (blocked never reaches Completed). Rewrite the changelog fragment that still described the superseded write-side clamp instead of the reader-side gate that actually ships. Correct a test doc comment that overclaimed which function the tool's write path shares with the simulation. Pause the clock in three new tests that were sleeping through real TICK_INTERVAL delays. Restore a doc comment line broken across three lines by the previous reflow. * test(kernel): make the status-bypass regression test actually discriminate The prior version passed identically against the commit it was meant to guard against status="completed" bypassing the verifier gate: a successful tick always overwrites goal.status back to InProgress at the end of the same iteration (new_status is never None on that path), so the tool-written Completed never survived to the next header check regardless of whether the header gated on it. The real window is a turn that calls the tool and then fails, since the Err arm never touches the goal document at all. The first tick now fails after writing, which makes the test red against the parent commit's header check and green against this one — confirmed by hand-reverting the header condition, running the test, and reverting back. * fix(goals): give the operator a real stop, and a breaker to every gate leg Six findings from the #7785 re-review. The verifier gate landed in the reader, which closed the `goal_update` tool's bypass but also took the operator's completion path with it — a `PUT` marking a gated goal `completed` no longer stopped the run, and the iteration in flight wrote it back to `in_progress`. Rather than guess which writer produced a stored value, route the operator through the run's real control channel: `update_goal_by_id` now calls `stop_goal_run` the way `delete_goal` always has, and the runner stops writing its own status and progress over a goal whose run has been stopped. The shared top-of-loop condition is unchanged, so #7973 stays converged. The verifier's dispatch errors never reached `classify_tick_error`, so a throttled verifier was reported as a dead one and `RateLimited` never fired for that leg; the rework turn had no breaker at all and could run to the iteration cap. Each leg now carries its own `GateStreak`, split by kind — sharing one counter between legs, or reusing the generator's `error_streak`, produces a counter that is reset or never read. Learnings were appended before the gate and again per rework round with no dedup, so a restated lesson was stored once per round; they now follow the same replace-the-rejected-reply rule as `parsed`, and the run refuses a text it already holds. Both goal endpoints now reject `verify_agent_id == agent_id`, update comparing the effective post-update pair, and the dashboard picker no longer offers the assigned agent. Create drops a blank `evaluator_model` instead of storing "", which is what an earlier review reply claimed it already did. Every test was run against the reverted production block first and recorded failing. * test(api): cover the operator stop at the injection site, not just the helper The kernel test covers the runner's half of the interlock; nothing covered the wiring itself, so deleting `stop_goal_run` from `update_goal_by_id` left the suite green. Four `TestServer` tests: start a run, `PUT` the terminal status, then read back. The follow-up `POST /stop` asserting `stopped: false` is the mechanism-precise part — the registry entry is removed synchronously during the `PUT`, so a second stop finds nothing left. Covers `cancelled` as well as `completed`, and the no-verifier path alongside the gated one that the review reported. The fourth is the negative: an ordinary edit must leave the run alone, which is what stops the interlock from being wired unconditionally. * fix(goals): let a bare operator stop keep the iteration's accounting goal_runner.rs read a plain AtomicBool and skipped patch_goal on any stop, so the two operator paths were treated as one when they are opposites. A terminal PUT /api/goals/{id} writes the document before stopping, and the runner's own end-of-iteration write is exactly what would revert it: new_status is InProgress for every iteration that did not pass verification, and the PUT carries progress too. That write has to be barred. A POST /api/goals/{id}/stop writes nothing. There is no operator choice on the document to protect, so skipping the write only throws away accounting already paid for: the goal keeps the previous iteration's progress while the run row reports the higher iteration count those turns were billed at. Two numbers that contradict each other, with nothing telling the operator which one counts. StopFlag carries who raised it, and stop_after_goal_write is a separate entry point taken only by the terminal PUT. Deletion keeps the plain stop: the rows are already gone, so an in-flight patch_goal updates nothing. wrote_goal is stored before stopped so a thread observing the stop also observes ownership; the reverse order leaves the exact window this distinction exists to close. Verified in red by restoring the previous semantics: the bare-stop test fails with progress 0 against the expected 60, and the terminal-PUT direction alone would have passed either way. * feat(goals): add pause/resume and a configurable loop cadence - Add `Paused` variant to `GoalRunPhase` so a suspended run preserves its iteration count and progress for later resumption. - `tick_interval_secs` on the goal document lets an operator tune the delay between loop iterations (default 2s, clamped 1–86400). - POST /api/goals/{id}/pause — cooperative pause; the in-flight turn finishes, then checkpoints and exits. - POST /api/goals/{id}/resume — continues from the checkpoint; refuses with 409 when there is nothing to resume. - Pause checkpoints are stored in the shared KV store and cleared on resume and on stop, so a stale checkpoint never seeds a fresh start. - Integration tests cover pause-signals-a-live-run, idle-goal-reports- false, and resume-without-pause-is-conflict. Refs #5744 * fix(goals): pass substrate to GoalRunner in tests * fix(goals): resume a paused run under its own cap and timestamps `POST /api/goals/{id}/resume` dispatched with no body, so the kernel substituted `DEFAULT_GOAL_MAX_ITERATIONS` for the cap the paused run was actually under. `GoalRunner::start` seeded the iteration count from the checkpoint while taking the cap from its argument, so a run started with 100 and paused at iteration 30 resumed under a cap of 25 — past the loop's own guard before its first turn, and the exit that follows clears the checkpoint, so the progress the resume was asked to continue was unrecoverable. The cap is now resolved in `GoalRunner::start`, the only layer that knows whether this is a resume: an explicit value wins, then the checkpoint's, then the compiled default. `/resume` accordingly accepts the same optional body `/start` does, and an explicit `max_iterations` there is a deliberate re-budgeting of the remaining run rather than something to ignore. A paused run's `paused_at` was written to the checkpoint and never read back, and `state()` stamped both timestamps with the clock, so two consecutive `GET /api/goals/{id}/run` on a motionless goal disagreed and "how long has this been paused" computed to roughly zero. `ResumePoint` now carries `started_at` and `paused_at`, the checkpoint stores the run's real start time, and both `state()` and `start()` restore them: a resume continues the run it checkpointed rather than opening a new segment, so it keeps the moment it actually began. A checkpoint predating these fields still resumes, falling back to the clock and the default cap. Also drops `goal_tick_sender_context`'s `display_name` parameter, which every caller filled with the same constant the function already hardcodes into `channel`. Tests: five `#[tokio::test]` cases against the real router drive a resume through `POST /resume` from a seeded checkpoint and pin the cap, the override, its validation, and both timestamps; three kernel tests pin the checkpoint round-trip and the cap precedence at the layer that decides it. * feat(goals): gate autonomous runs on a verifier and an evaluator A goal run ended the moment the agent wrote GOAL_DONE, which made the worker the sole judge of its own work - the one check a long-horizon loop most needs, and the one it did not have. Setting loop_engineering on a goal adds two judges that are not the worker. A verifier agent reads each iteration's output and returns VERDICT: PASS|FAIL|NEEDS_REWORK; a rejection goes back to the generator carrying the verifier's stated reason, up to verify_max_retries rework rounds, and until the verifier passes the work GOAL_DONE does not end the run. An evaluator model makes one cheap yes/no read of the goal against the latest output and can conclude the goal is met even when the agent never claimed it. The agent can also record a reusable lesson with GOAL_LEARNED: <text>. Captured lessons are replayed into later iterations' prompts, persisted under goal_learnings_<id>, and written into a goal-learned-* skill through the same prompt-injection scan every other skill-creation path uses. All three are inert unless the goal opts in: with loop_engineering off the prompt is byte-identical to the one it was and the run still makes exactly one LLM call per iteration. a_plain_run_makes_no_extra_llm_calls asserts that, because an evaluator that fires on every goal would double every existing operator's bill silently. Sub-agents are delegated, not provisioned. The prompt directs the agent to its own agent_spawn / agent_send tools, which run under the capabilities its operator granted; neither the runner nor the API ever creates an agent on a caller's behalf. loop_engineering_without_a_verifier_provisions_no_agent and goal_run_start_with_an_unusable_agent_id_provisions_no_agent assert the registry does not grow, which is the property that survives a later refactor - asserting only the status code would not. An unusable verify_agent_id is a 400 with the id named rather than a silently ignored field, because dropping it downgrades a gated run to an ungated one without telling the operator who configured the gate. A run now also gives up after five consecutive non-rate-limit tick failures instead of spending its whole iteration budget rediscovering a deleted agent or a revoked key. Refs #6505 * fix(goals): register a goal run's handle before spawning its loop start() spawned the loop task and inserted its RunHandle afterwards. A loop that finishes inside that window runs its self-cleanup remove_if against a registry that does not hold it yet: the removal finds nothing, the insert then lands a handle for a run that is already over, and nothing ever collects it. state() reports the run as present forever and the registry grows by one every time it happens. The window is short, but the exits that fit inside it are the fast ones - a pre-signalled shutdown, or a goal deleted between the API's read and this call - both of which end the loop before its first agent turn. The handle is now registered before the spawn and the JoinHandle backfilled after. A missing entry at backfill time means the loop already finished and cleaned up, so dropping the handle there is correct: there is nothing left to abort. The generation check keeps a replacement run's handle from being overwritten, and stop() cannot interleave because it takes start_lock, which start() holds for the whole sequence. a_run_that_ends_immediately_leaves_no_entry_behind pre-signals shutdown so the loop breaks on its first check - the shortest path from spawn to remove_if - and runs many rounds on a multi-threaded runtime. It probes the registry directly rather than through state(), which answers None both for "no entry" and for "the state lock was momentarily held" and would otherwise read a transient lock as a clean registry. Hitting the race is probabilistic; the invariant is not. With the handle registered before the spawn there is no ordering in which a finished loop leaves an entry behind, so the test cannot fail spuriously. Pre-existing upstream bug, carried in this PR because it lives in the function the loop-engineering change rewrites. * test(goals): assert on the registry's own agent name AgentEntry carries both `name` (what the registry holds) and `manifest.name` (what the caller asked for). The two diverge when a spawn path renames for uniqueness, and the property under test is what the registry actually ended up holding, so `name` is the field that answers it. `list_arcs` sorts on the same field. * docs(changelog): point the fragment at this PR's number The trailing (#N) group is what the release flow matches against the generated line for this PR, so it has to be #7785. #6505 stays as a mid-bullet cross-reference, which the tooling ignores by design. * fix(dashboard): remove unnecessary escape in goals test * fix(goals): restore goal_run_start signature lost during rebase * fix(goals): sync start_goal_run call with upstream 7-arg signature Move the kernel call after parameter extraction so loop-engineering fields (verify_agent_id, verify_max_retries, evaluator_model) are resolved before they are passed. Update the test call site to match. * fix(goals): resolve all rebase artifacts in loopeng branch Restore kernel_api.rs from origin/main and apply the loop engineering parameter additions on top — the prior rebase replaced the entire impl block with self-recursive default methods, producing 133 compilation errors. Move loop-engineering parameter extraction in routes/goals.rs before the start_goal_run call so the new 7-arg signature is satisfied. * fix(goals): add missing args to goal_runner test call site The start() method takes 11 args after the loop-engineering addition. The handle-unset test was still passing only 5. * fix(goals): update test call sites to match 11-arg start() signature Two tests still used the old 5-arg start() signature after the verifier gate was added. Update both call sites to pass the six new arguments (on_learnings_captured, evaluate_goal, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model) and remove the duplicate test that was added during rebase. * fix(goals): send GOAL_LEARNED lessons to the pending approval queue A goal run wrote its captured lessons straight into the installed skills directory, gated only by the goal's `loop_engineering` flag. That was a second path to skill creation that approved itself, standing next to a workshop (#3328) that deliberately requires an explicit `librefang skill pending approve` before any machine-proposed skill reaches an agent's prompt — and it was reachable by an agent emitting one marker line in its own output. The prompt-injection scan did run, but that is only half the boundary; the other half is that a human has read the thing. The lessons now become a `CandidateSkill` in `pending/<agent>/`, the same queue the background skill reviewer already files into, and only approval installs them. A second run of the same goal files a `CandidateKind::Update` draft when the first run's skill is already installed, so approval routes through `update_skill` rather than failing on the name already existing and dropping what the second run learned. Cap and TTL come from the producing agent's `[skill_workshop]` block, read once at run start. Drafts that did not come from a conversation turn are tagged with a sentinel `explicit_instruction` trigger naming the producer, the convention the reviewer already uses for `auto_evolve_reviewer`; this one is `goal_learned`. Nothing about the durable record changes: the runner still writes `goal_learnings_<id>` to the shared store before any of this, so a draft that is capped out or rejected costs the agent a convenience, not the lessons. * docs(goals): say what an unresolvable evaluator_model does, and give the tick breaker its own entry `verify_agent_id` is validated at save time and `evaluator_model` is not, which reads as an oversight until the asymmetry is written down. A verifier id has a checkable shape and no correct value that fails the check; a model id has neither, and whether one resolves depends on the provider configuration at the moment of the call rather than at save time. So a save-time 400 would reject a model the operator is about to configure and would still not guarantee the id resolves when the run reaches it. The behaviour that is already in place is the right one — a `WARN` per iteration and a fall back to the agent's own marker — and it is now stated on the field and next to the route that stores it. The five-consecutive-failure circuit breaker moves out of the loop-engineering fragment's last line into its own entry, because it is a fix an operator recognises on its own terms: a run pointed at a deleted agent now ends in `Stopped` carrying the error instead of in `MaxIterationsReached` carrying nothing. * fix(dashboard): the run-phase union and the badge must know about "paused" GoalRunState.phase is "typed from the wire contract rather than string so a caller cannot pass a typo'd literal" — but this PR's own server now sends "paused", a value the type said could not exist. The badge then fell through to the default styling and GoalsPage rendered a bare untranslated "paused" in every language, while every other phase had a locale key. The union gains "paused", the badge gains a case (warning palette — the run is alive but not progressing, same family as stopped), and goals.run_phase_paused lands in all five locales. The pause/resume dashboard client (pauseGoalRun / resumeGoalRun wrappers and their mutation hooks) is deliberately not here: it arrives in #8029, which is stacked on this branch and adds the controls that use it. * fix(kernel): goal runner resume, verifier gate, and pause correctness Four defects the maintainer review of #7973 found unaddressed after the review pass: - run_loop counted iterations from a hardcoded 0 instead of the checkpointed value start() already resolved into the run's state, so a resumed run got a fresh iteration budget rather than continuing the one it was paused under. - A verifier-rejected iteration's unclamped GOAL_PROGRESS: 100 let the next iteration's plain progress>=100 check finish the run anyway, bypassing the verifier gate entirely. - Pause/stop were only observed at the top of the outer loop, so a tick_interval_secs near its 24h ceiling left a pause request unobserved for up to a day; the inter-tick sleep now polls every second instead of sleeping the whole interval in one shot. - Pausing discarded whatever GOAL_LEARNED: lessons the run had captured so far. The resume checkpoint now carries them and threads them back into the resumed run's own accumulator. * fix(kernel): goal run learnings respect the skill workshop opt-in queue_learnings_as_pending_skill was called unconditionally from the goal runner's on_learnings hook, so a goal run queued a pending skill draft even for agents that never set [skill_workshop] enabled = true in agent.toml. Nothing else in the goal-run path consulted the workshop config -- only the approval-side CLI/API/dashboard did. Extracted the gate into should_queue_learnings (enabled && auto_capture), matching the check every other automatic capture path in the workshop already applies. * fix(api): validate verify_max_retries and blank verifier/evaluator fields start_or_resume read verify_max_retries with a bare .map(|n| n as u32), so an out-of-range or wrongly-typed value silently wrapped instead of 400ing -- the same field max_iterations already validates properly a few lines above. create_goal's verify_agent_id parsing didn't go through optional_uuid_field, so a blank string (the create form's clear signal, per #6562) 400ed as "Invalid verify_agent_id" instead of being treated as unset. evaluator_model_str had no blank filter either, so a blank string was stored verbatim as Some(""), contrary to the field's own documented None-means-unconfigured contract. * fix(cli): goal --watch recognizes the paused run phase terminal_phase_message had no branch for "paused", so a goal that transitioned into paused mid-watch (an operator pausing it from the dashboard, say) read as an unclassified state: classify_poll routed it to Unobservable, burning the bounded retry budget before --watch gave up with a generic "outcome unknown" message instead of reporting that the run was paused. Added the branch plus a locale key in all four CLI locales, and extended every_terminal_phase_has_a_summary to cover it. * fix(kernel): close the goal_update verifier-gate bypass and harden pause/resume /review of 56e75aee0..562f414b5 found the progress clamp from the first pass protected only one of goal.progress's two writers: goal_update (the tool the agent's own system prompt tells it to call) patches the same shared document directly, bypassing parse_tick entirely, so a rejected iteration could still close the run one tick later through the bare top-of-loop progress>=100 / status==Completed check. PR #7785 found and fixed the identical bug first; adopted its reader-side gate verbatim (bare progress/status only count as done when no verifier is configured; Cancelled stays unconditioned) so the two PRs converge instead of one reverting the other on merge, and kept the clamp as defense in depth scoped to !verified. Also from the same review: - The inter-tick sleep now wakes to a fixed deadline via sleep_until instead of chaining PAUSE_POLL_INTERVAL sleeps end-to-end, which would have drifted by scheduling latency accumulated once per slice (on the order of a minute over the 24h maximum tick interval). - verify_max_retries now survives a pause the same way max_iterations already does: added to the resume checkpoint with the same argument-then-checkpoint-then-default precedence. - run_loop_resumes_from_the_checkpointed_iteration_not_zero passed identically with and without the iteration-reset fix it was meant to guard, because the final iteration count at MaxIterationsReached is the same either way. Now counts turns and inspects the first resumed prompt, which does discriminate. - learnings_survive_a_pause_and_resume_cycle asserted an exact ordered list that only held if exactly one tick landed before pause() took effect; a second tick under load duplicated a fixed lesson. The send closure now returns a distinct lesson per turn and the assertion checks membership/uniqueness instead of an exact list. - Added a regression test for pause and shutdown signalled together mid-sleep: both now resolve through the same pause-before-shutdown check at the top of the loop, so they land on Paused rather than the old flat select! hardcoding Stopped in that one path. * fix(kernel): move the skill-workshop opt-in gate into its single callee /review of 83e9bfecf found should_queue_learnings gated only the caller (goal_run_start's on_learnings closure), leaving queue_learnings_as_pending_skill's own two pre-existing tests untested for the opt-in -- both called it directly with the disabled default and asserted a draft got queued, which is exactly what the fix was meant to stop. Moved the gate inside the function itself (its only production caller), updated those two tests to enabled: true since they were asserting the pre-fix behavior, and added a test asserting a disabled workshop queues nothing when called directly. * fix(api): refuse a resume cap at or below the checkpoint's iteration /review of 56e75aee0 found a side effect of the iteration-reset fix: GoalRunner::start now compares an explicit max_iterations against the RESTORED iteration count instead of 0, so a resume body naming a cap at or below that count (e.g. {"max_iterations": 10} on a run paused at iteration 30) trips the cap check on the very first pass with no turn run, and clears the checkpoint -- and the learnings it carries -- on the way out, for a request that could never have advanced the run. start_or_resume now reads the goal's run state once (reused for the existing require_paused check) and rejects such a cap with a 400 naming both numbers, on both /start and /resume since /start also auto-resumes from an existing checkpoint. * fix(kernel): correct stale doc comments and cover the registry-absent case /review of 378b8bfa4 found two loose ends: - The doc comments on GoalRunner::start and its test described an explicit resume max_iterations as "re-budgeting the remaining run", which is not what the code does or what the earlier fix in this branch made it do: the cap is a TOTAL ceiling compared against the restored iteration count, exactly like a fresh start compares against 0. Corrected both, and noted the new below-checkpoint 400 in the API route's own doc comment. - should_queue_learnings' own tests only exercised it as a pure predicate; nothing verified the other half of goal_run_start's wiring, that an agent absent from the registry (deleted, never spawned, mistyped id) still resolves to a denying config rather than some other default. Extracted that resolution into resolve_workshop_config so it's callable without a live turn, and added two tests: absent-agent denies, and a registered agent's own manifest setting is what actually gets read (proving the first test isn't passing because the function ignores the registry outright). Verified the new test actually guards something: temporarily made the fallback permissive (enabled: true instead of ::default()) and confirmed resolve_workshop_config_denies_when_the_agent_is_absent_from_the_registry fails before restoring it. * fix(kernel): cancel discards a pause checkpoint it cannot read `load_pause_checkpoint` answers `None` for two different things: there is no checkpoint, and there is a row it could not turn into a `ResumePoint`. A substrate read error takes …

…ng surfaces (#8230) * feat(dashboard): add pause/resume controls to the goals page Wire up the pause and resume goal mutations in GoalsPage so users can pause a running goal and resume a paused one. Adds "paused" state badge, Pause/Resume buttons, and i18n keys. * fix(dashboard): add goal pause/resume i18n keys to ko/pl and fix test mocks * feat(goals): add pause/resume and a configurable loop cadence - Add `Paused` variant to `GoalRunPhase` so a suspended run preserves its iteration count and progress for later resumption. - `tick_interval_secs` on the goal document lets an operator tune the delay between loop iterations (default 2s, clamped 1–86400). - POST /api/goals/{id}/pause — cooperative pause; the in-flight turn finishes, then checkpoints and exits. - POST /api/goals/{id}/resume — continues from the checkpoint; refuses with 409 when there is nothing to resume. - Pause checkpoints are stored in the shared KV store and cleared on resume and on stop, so a stale checkpoint never seeds a fresh start. - Integration tests cover pause-signals-a-live-run, idle-goal-reports- false, and resume-without-pause-is-conflict. Refs #5744 * fix(goals): pass substrate to GoalRunner in tests * fix(dashboard): goal pause/resume controls track the run-level phase and drop dead fallbacks Address #8029 review feedback: - Drop the goal-status "paused" arms from progressForGoalStatus and goalStatusBadgeVariant: pause is a run-level phase (GoalRunPhase::Paused from #7973), and GoalStatus never emits "paused", so those arms were unreachable and modelled the wrong concept. - Remove the dead defaultValue fallbacks from run_pause, run_resume and run_stop — all three keys exist and are translated in every locale, and a present default hides a future missing-key regression. - Add a translated run_phase_paused label to all five locales and a paused arm to the run-phase pill, which the stacked backend makes reachable. - Cover the pause and resume buttons with tests asserting the mutations fire with the goal id. - Add the changelog fragment. The branch is stacked on #7973, which supplies POST /api/goals/{id}/pause and /resume and GoalRunPhase::Paused. * fix(goals): reject a pause checkpoint whose max_iterations is missing load_pause_checkpoint parsed agent_id with `?` but read max_iterations with unwrap_or(0), answering the same corrupt-checkpoint question two different ways. A cap of zero is not a state an operator can reach: the API rejects max_iterations: 0 as a bad request and goal_run_start clamps it up with .max(1), so a partial checkpoint surfaced through GoalRunner::state as a paused run reporting a cap nobody chose. Treat it like agent_id and fall through to None, which puts a damaged checkpoint on the restart path an unreadable one already takes. iteration and last_progress keep unwrap_or(0) because zero is a real value for both. * fix(goals): resume a paused run under its own cap and timestamps `POST /api/goals/{id}/resume` dispatched with no body, so the kernel substituted `DEFAULT_GOAL_MAX_ITERATIONS` for the cap the paused run was actually under. `GoalRunner::start` seeded the iteration count from the checkpoint while taking the cap from its argument, so a run started with 100 and paused at iteration 30 resumed under a cap of 25 — past the loop's own guard before its first turn, and the exit that follows clears the checkpoint, so the progress the resume was asked to continue was unrecoverable. The cap is now resolved in `GoalRunner::start`, the only layer that knows whether this is a resume: an explicit value wins, then the checkpoint's, then the compiled default. `/resume` accordingly accepts the same optional body `/start` does, and an explicit `max_iterations` there is a deliberate re-budgeting of the remaining run rather than something to ignore. A paused run's `paused_at` was written to the checkpoint and never read back, and `state()` stamped both timestamps with the clock, so two consecutive `GET /api/goals/{id}/run` on a motionless goal disagreed and "how long has this been paused" computed to roughly zero. `ResumePoint` now carries `started_at` and `paused_at`, the checkpoint stores the run's real start time, and both `state()` and `start()` restore them: a resume continues the run it checkpointed rather than opening a new segment, so it keeps the moment it actually began. A checkpoint predating these fields still resumes, falling back to the clock and the default cap. Also drops `goal_tick_sender_context`'s `display_name` parameter, which every caller filled with the same constant the function already hardcodes into `channel`. Tests: five `#[tokio::test]` cases against the real router drive a resume through `POST /resume` from a seeded checkpoint and pin the cap, the override, its validation, and both timestamps; three kernel tests pin the checkpoint round-trip and the cap precedence at the layer that decides it. * feat(goals): gate autonomous runs on a verifier and an evaluator A goal run ended the moment the agent wrote GOAL_DONE, which made the worker the sole judge of its own work - the one check a long-horizon loop most needs, and the one it did not have. Setting loop_engineering on a goal adds two judges that are not the worker. A verifier agent reads each iteration's output and returns VERDICT: PASS|FAIL|NEEDS_REWORK; a rejection goes back to the generator carrying the verifier's stated reason, up to verify_max_retries rework rounds, and until the verifier passes the work GOAL_DONE does not end the run. An evaluator model makes one cheap yes/no read of the goal against the latest output and can conclude the goal is met even when the agent never claimed it. The agent can also record a reusable lesson with GOAL_LEARNED: <text>. Captured lessons are replayed into later iterations' prompts, persisted under goal_learnings_<id>, and written into a goal-learned-* skill through the same prompt-injection scan every other skill-creation path uses. All three are inert unless the goal opts in: with loop_engineering off the prompt is byte-identical to the one it was and the run still makes exactly one LLM call per iteration. a_plain_run_makes_no_extra_llm_calls asserts that, because an evaluator that fires on every goal would double every existing operator's bill silently. Sub-agents are delegated, not provisioned. The prompt directs the agent to its own agent_spawn / agent_send tools, which run under the capabilities its operator granted; neither the runner nor the API ever creates an agent on a caller's behalf. loop_engineering_without_a_verifier_provisions_no_agent and goal_run_start_with_an_unusable_agent_id_provisions_no_agent assert the registry does not grow, which is the property that survives a later refactor - asserting only the status code would not. An unusable verify_agent_id is a 400 with the id named rather than a silently ignored field, because dropping it downgrades a gated run to an ungated one without telling the operator who configured the gate. A run now also gives up after five consecutive non-rate-limit tick failures instead of spending its whole iteration budget rediscovering a deleted agent or a revoked key. Refs #6505 * fix(goals): register a goal run's handle before spawning its loop start() spawned the loop task and inserted its RunHandle afterwards. A loop that finishes inside that window runs its self-cleanup remove_if against a registry that does not hold it yet: the removal finds nothing, the insert then lands a handle for a run that is already over, and nothing ever collects it. state() reports the run as present forever and the registry grows by one every time it happens. The window is short, but the exits that fit inside it are the fast ones - a pre-signalled shutdown, or a goal deleted between the API's read and this call - both of which end the loop before its first agent turn. The handle is now registered before the spawn and the JoinHandle backfilled after. A missing entry at backfill time means the loop already finished and cleaned up, so dropping the handle there is correct: there is nothing left to abort. The generation check keeps a replacement run's handle from being overwritten, and stop() cannot interleave because it takes start_lock, which start() holds for the whole sequence. a_run_that_ends_immediately_leaves_no_entry_behind pre-signals shutdown so the loop breaks on its first check - the shortest path from spawn to remove_if - and runs many rounds on a multi-threaded runtime. It probes the registry directly rather than through state(), which answers None both for "no entry" and for "the state lock was momentarily held" and would otherwise read a transient lock as a clean registry. Hitting the race is probabilistic; the invariant is not. With the handle registered before the spawn there is no ordering in which a finished loop leaves an entry behind, so the test cannot fail spuriously. Pre-existing upstream bug, carried in this PR because it lives in the function the loop-engineering change rewrites. * test(goals): assert on the registry's own agent name AgentEntry carries both `name` (what the registry holds) and `manifest.name` (what the caller asked for). The two diverge when a spawn path renames for uniqueness, and the property under test is what the registry actually ended up holding, so `name` is the field that answers it. `list_arcs` sorts on the same field. * docs(changelog): point the fragment at this PR's number The trailing (#N) group is what the release flow matches against the generated line for this PR, so it has to be #7785. #6505 stays as a mid-bullet cross-reference, which the tooling ignores by design. * fix(dashboard): remove unnecessary escape in goals test * fix(goals): restore goal_run_start signature lost during rebase * fix(goals): sync start_goal_run call with upstream 7-arg signature Move the kernel call after parameter extraction so loop-engineering fields (verify_agent_id, verify_max_retries, evaluator_model) are resolved before they are passed. Update the test call site to match. * fix(goals): resolve all rebase artifacts in loopeng branch Restore kernel_api.rs from origin/main and apply the loop engineering parameter additions on top — the prior rebase replaced the entire impl block with self-recursive default methods, producing 133 compilation errors. Move loop-engineering parameter extraction in routes/goals.rs before the start_goal_run call so the new 7-arg signature is satisfied. * fix(goals): add missing args to goal_runner test call site The start() method takes 11 args after the loop-engineering addition. The handle-unset test was still passing only 5. * fix(goals): update test call sites to match 11-arg start() signature Two tests still used the old 5-arg start() signature after the verifier gate was added. Update both call sites to pass the six new arguments (on_learnings_captured, evaluate_goal, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model) and remove the duplicate test that was added during rebase. * fix(goals): send GOAL_LEARNED lessons to the pending approval queue A goal run wrote its captured lessons straight into the installed skills directory, gated only by the goal's `loop_engineering` flag. That was a second path to skill creation that approved itself, standing next to a workshop (#3328) that deliberately requires an explicit `librefang skill pending approve` before any machine-proposed skill reaches an agent's prompt — and it was reachable by an agent emitting one marker line in its own output. The prompt-injection scan did run, but that is only half the boundary; the other half is that a human has read the thing. The lessons now become a `CandidateSkill` in `pending/<agent>/`, the same queue the background skill reviewer already files into, and only approval installs them. A second run of the same goal files a `CandidateKind::Update` draft when the first run's skill is already installed, so approval routes through `update_skill` rather than failing on the name already existing and dropping what the second run learned. Cap and TTL come from the producing agent's `[skill_workshop]` block, read once at run start. Drafts that did not come from a conversation turn are tagged with a sentinel `explicit_instruction` trigger naming the producer, the convention the reviewer already uses for `auto_evolve_reviewer`; this one is `goal_learned`. Nothing about the durable record changes: the runner still writes `goal_learnings_<id>` to the shared store before any of this, so a draft that is capped out or rejected costs the agent a convenience, not the lessons. * docs(goals): say what an unresolvable evaluator_model does, and give the tick breaker its own entry `verify_agent_id` is validated at save time and `evaluator_model` is not, which reads as an oversight until the asymmetry is written down. A verifier id has a checkable shape and no correct value that fails the check; a model id has neither, and whether one resolves depends on the provider configuration at the moment of the call rather than at save time. So a save-time 400 would reject a model the operator is about to configure and would still not guarantee the id resolves when the run reaches it. The behaviour that is already in place is the right one — a `WARN` per iteration and a fall back to the agent's own marker — and it is now stated on the field and next to the route that stores it. The five-consecutive-failure circuit breaker moves out of the loop-engineering fragment's last line into its own entry, because it is a fix an operator recognises on its own terms: a run pointed at a deleted agent now ends in `Stopped` carrying the error instead of in `MaxIterationsReached` carrying nothing. * fix(dashboard): the run-phase union and the badge must know about "paused" GoalRunState.phase is "typed from the wire contract rather than string so a caller cannot pass a typo'd literal" — but this PR's own server now sends "paused", a value the type said could not exist. The badge then fell through to the default styling and GoalsPage rendered a bare untranslated "paused" in every language, while every other phase had a locale key. The union gains "paused", the badge gains a case (warning palette — the run is alive but not progressing, same family as stopped), and goals.run_phase_paused lands in all five locales. The pause/resume dashboard client (pauseGoalRun / resumeGoalRun wrappers and their mutation hooks) is deliberately not here: it arrives in #8029, which is stacked on this branch and adds the controls that use it. * fix(kernel): goal runner resume, verifier gate, and pause correctness Four defects the maintainer review of #7973 found unaddressed after the review pass: - run_loop counted iterations from a hardcoded 0 instead of the checkpointed value start() already resolved into the run's state, so a resumed run got a fresh iteration budget rather than continuing the one it was paused under. - A verifier-rejected iteration's unclamped GOAL_PROGRESS: 100 let the next iteration's plain progress>=100 check finish the run anyway, bypassing the verifier gate entirely. - Pause/stop were only observed at the top of the outer loop, so a tick_interval_secs near its 24h ceiling left a pause request unobserved for up to a day; the inter-tick sleep now polls every second instead of sleeping the whole interval in one shot. - Pausing discarded whatever GOAL_LEARNED: lessons the run had captured so far. The resume checkpoint now carries them and threads them back into the resumed run's own accumulator. * fix(kernel): goal run learnings respect the skill workshop opt-in queue_learnings_as_pending_skill was called unconditionally from the goal runner's on_learnings hook, so a goal run queued a pending skill draft even for agents that never set [skill_workshop] enabled = true in agent.toml. Nothing else in the goal-run path consulted the workshop config -- only the approval-side CLI/API/dashboard did. Extracted the gate into should_queue_learnings (enabled && auto_capture), matching the check every other automatic capture path in the workshop already applies. * fix(api): validate verify_max_retries and blank verifier/evaluator fields start_or_resume read verify_max_retries with a bare .map(|n| n as u32), so an out-of-range or wrongly-typed value silently wrapped instead of 400ing -- the same field max_iterations already validates properly a few lines above. create_goal's verify_agent_id parsing didn't go through optional_uuid_field, so a blank string (the create form's clear signal, per #6562) 400ed as "Invalid verify_agent_id" instead of being treated as unset. evaluator_model_str had no blank filter either, so a blank string was stored verbatim as Some(""), contrary to the field's own documented None-means-unconfigured contract. * fix(cli): goal --watch recognizes the paused run phase terminal_phase_message had no branch for "paused", so a goal that transitioned into paused mid-watch (an operator pausing it from the dashboard, say) read as an unclassified state: classify_poll routed it to Unobservable, burning the bounded retry budget before --watch gave up with a generic "outcome unknown" message instead of reporting that the run was paused. Added the branch plus a locale key in all four CLI locales, and extended every_terminal_phase_has_a_summary to cover it. * fix(kernel): close the goal_update verifier-gate bypass and harden pause/resume /review of 56e75aee0..562f414b5 found the progress clamp from the first pass protected only one of goal.progress's two writers: goal_update (the tool the agent's own system prompt tells it to call) patches the same shared document directly, bypassing parse_tick entirely, so a rejected iteration could still close the run one tick later through the bare top-of-loop progress>=100 / status==Completed check. PR #7785 found and fixed the identical bug first; adopted its reader-side gate verbatim (bare progress/status only count as done when no verifier is configured; Cancelled stays unconditioned) so the two PRs converge instead of one reverting the other on merge, and kept the clamp as defense in depth scoped to !verified. Also from the same review: - The inter-tick sleep now wakes to a fixed deadline via sleep_until instead of chaining PAUSE_POLL_INTERVAL sleeps end-to-end, which would have drifted by scheduling latency accumulated once per slice (on the order of a minute over the 24h maximum tick interval). - verify_max_retries now survives a pause the same way max_iterations already does: added to the resume checkpoint with the same argument-then-checkpoint-then-default precedence. - run_loop_resumes_from_the_checkpointed_iteration_not_zero passed identically with and without the iteration-reset fix it was meant to guard, because the final iteration count at MaxIterationsReached is the same either way. Now counts turns and inspects the first resumed prompt, which does discriminate. - learnings_survive_a_pause_and_resume_cycle asserted an exact ordered list that only held if exactly one tick landed before pause() took effect; a second tick under load duplicated a fixed lesson. The send closure now returns a distinct lesson per turn and the assertion checks membership/uniqueness instead of an exact list. - Added a regression test for pause and shutdown signalled together mid-sleep: both now resolve through the same pause-before-shutdown check at the top of the loop, so they land on Paused rather than the old flat select! hardcoding Stopped in that one path. * fix(kernel): move the skill-workshop opt-in gate into its single callee /review of 83e9bfecf found should_queue_learnings gated only the caller (goal_run_start's on_learnings closure), leaving queue_learnings_as_pending_skill's own two pre-existing tests untested for the opt-in -- both called it directly with the disabled default and asserted a draft got queued, which is exactly what the fix was meant to stop. Moved the gate inside the function itself (its only production caller), updated those two tests to enabled: true since they were asserting the pre-fix behavior, and added a test asserting a disabled workshop queues nothing when called directly. * fix(api): refuse a resume cap at or below the checkpoint's iteration /review of 56e75aee0 found a side effect of the iteration-reset fix: GoalRunner::start now compares an explicit max_iterations against the RESTORED iteration count instead of 0, so a resume body naming a cap at or below that count (e.g. {"max_iterations": 10} on a run paused at iteration 30) trips the cap check on the very first pass with no turn run, and clears the checkpoint -- and the learnings it carries -- on the way out, for a request that could never have advanced the run. start_or_resume now reads the goal's run state once (reused for the existing require_paused check) and rejects such a cap with a 400 naming both numbers, on both /start and /resume since /start also auto-resumes from an existing checkpoint. * fix(kernel): correct stale doc comments and cover the registry-absent case /review of 378b8bfa4 found two loose ends: - The doc comments on GoalRunner::start and its test described an explicit resume max_iterations as "re-budgeting the remaining run", which is not what the code does or what the earlier fix in this branch made it do: the cap is a TOTAL ceiling compared against the restored iteration count, exactly like a fresh start compares against 0. Corrected both, and noted the new below-checkpoint 400 in the API route's own doc comment. - should_queue_learnings' own tests only exercised it as a pure predicate; nothing verified the other half of goal_run_start's wiring, that an agent absent from the registry (deleted, never spawned, mistyped id) still resolves to a denying config rather than some other default. Extracted that resolution into resolve_workshop_config so it's callable without a live turn, and added two tests: absent-agent denies, and a registered agent's own manifest setting is what actually gets read (proving the first test isn't passing because the function ignores the registry outright). Verified the new test actually guards something: temporarily made the fallback permissive (enabled: true instead of ::default()) and confirmed resolve_workshop_config_denies_when_the_agent_is_absent_from_the_registry fails before restoring it. * fix(api): resume answers 404 for a goal that does not exist The `require_paused` precondition ran between `parse_goal_id` and the substrate read that resolves the goal, so `POST /api/goals/{id}/resume` answered a well-formed but unknown id with 409 and the advice to use `POST /api/goals/{id}/start` instead — the same handler body with `require_paused: false`, which reaches the lookup and would itself have answered 404. An operator who mistyped an id was told the wrong thing and pointed at a request that could not work. Move the run-state read and the precondition below the goal lookup. The 409 is unchanged for its real case, a goal that exists with nothing paused, and it still precedes the agent_id validation so no other status moves. `/stop`, `/pause` and `/run` are deliberately not part of this: none of them looks the goal up, and all three answer 200 with a false flag for an unknown id. * fix(kernel): cancel discards a pause checkpoint it cannot read `load_pause_checkpoint` answers `None` for two different things: there is no checkpoint, and there is a row it could not turn into a `ResumePoint`. A substrate read error takes the second path through its `.ok().flatten()?`, and so does a row whose `agent_id` is missing or not a UUID. `stop_locked` gated the delete on that `None`, so in either case it skipped the delete and left the row behind — and the next `/start` auto-resumes from a checkpoint, so a transient storage failure at cancel time silently resurrected the run the operator had just cancelled. That is the outcome the comment directly above the delete says cancel exists to prevent. Delete unconditionally and use the read only for the return value, which still reports whether a resume point was readable. Costs nothing: `clear_pause_checkpoint` was already a no-op on a missing key. The test asserts the raw key rather than `load_pause_checkpoint`, which is `None` before and after the delete on this input and so cannot tell the fix from the bug. * test(dashboard): move the unknown-phase example off "paused" #8067 used `paused` as its stand-in for a phase the daemon emits before the dashboard knows it, which is exactly the gap this PR closes. With `paused` now carrying its own switch arm the example had to become a phase the switch still does not know, and `paused` joins the API-emittable list with a test asserting its own variant rather than the neutral fallback. * fix(goals): restore the resume path's loop-engineering arguments Rebasing this branch onto `origin/main` flattens the twenty-five merge commits it accumulated, and a flattened replay resolves each commit against its own parent instead of against the resolution the merge already chose. Three of those resolutions did not survive the replay, and the first one does not compile. - `KernelApi::resume_goal_run` and `LibreFangKernel::goal_run_resume` came back with their original three arguments while `routes/goals.rs` calls them with seven. A resume that does not carry `loop_engineering` / `verify_agent_id` / `verify_max_retries` / `evaluator_model` silently drops the verifier gate the operator configured on the goal, which is the failure the gate exists to prevent. - `GoalRunner::start` lost the block that reconstructs those four values from the goal document when a run is resumed, so a resumed run reported no verifier even when the goal has one. - `ko.json` ended up with `goals.run_phase_paused` twice. Both spellings parse, the later one wins, and nothing warns. Each file is restored to the three-way merge of this branch's own tip against `origin/main`, so what lands is the resolution the branch already made rather than a fresh guess at it. * fix(kernel): a paused run reported the default verifier retry budget `GoalRunner::state` reconstructs a paused run from its checkpoint once the loop task has exited and self-cleaned its registry slot, and it restored every field of that run from the checkpoint except the verifier retry budget, which it took from the compiled default instead. A run started with `{"verify_max_retries": 8}` therefore answered `GET /api/goals/{id}/run` with 3 while it was paused, and the bodyless `/resume` that follows went back to 8 — `GoalRunner::start` already resolves the checkpoint's value ahead of the default. The readout and the resume disagreed about the budget the run was under, and the resume was the one telling the truth. `ResumePoint::verify_max_retries` names this exact surface as the reason the field is carried at all, and the sibling `max_iterations` two fields up already reads the checkpoint, so this restores the field to the behaviour its own documentation describes rather than introducing a new rule. The `loop_engineering` gate is unchanged: a goal with the flag off still reports no budget, because reporting one would advertise a gate the resume will not apply. Both directions are covered — the restore, and the gate that must keep answering zero without loop engineering. * feat(goals): gate autonomous runs on a verifier and an evaluator A goal run ended the moment the agent wrote GOAL_DONE, which made the worker the sole judge of its own work - the one check a long-horizon loop most needs, and the one it did not have. Setting loop_engineering on a goal adds two judges that are not the worker. A verifier agent reads each iteration's output and returns VERDICT: PASS|FAIL|NEEDS_REWORK; a rejection goes back to the generator carrying the verifier's stated reason, up to verify_max_retries rework rounds, and until the verifier passes the work GOAL_DONE does not end the run. An evaluator model makes one cheap yes/no read of the goal against the latest output and can conclude the goal is met even when the agent never claimed it. The agent can also record a reusable lesson with GOAL_LEARNED: <text>. Captured lessons are replayed into later iterations' prompts, persisted under goal_learnings_<id>, and written into a goal-learned-* skill through the same prompt-injection scan every other skill-creation path uses. All three are inert unless the goal opts in: with loop_engineering off the prompt is byte-identical to the one it was and the run still makes exactly one LLM call per iteration. a_plain_run_makes_no_extra_llm_calls asserts that, because an evaluator that fires on every goal would double every existing operator's bill silently. Sub-agents are delegated, not provisioned. The prompt directs the agent to its own agent_spawn / agent_send tools, which run under the capabilities its operator granted; neither the runner nor the API ever creates an agent on a caller's behalf. loop_engineering_without_a_verifier_provisions_no_agent and goal_run_start_with_an_unusable_agent_id_provisions_no_agent assert the registry does not grow, which is the property that survives a later refactor - asserting only the status code would not. An unusable verify_agent_id is a 400 with the id named rather than a silently ignored field, because dropping it downgrades a gated run to an ungated one without telling the operator who configured the gate. A run now also gives up after five consecutive non-rate-limit tick failures instead of spending its whole iteration budget rediscovering a deleted agent or a revoked key. Refs #6505 * fix(goals): register a goal run's handle before spawning its loop start() spawned the loop task and inserted its RunHandle afterwards. A loop that finishes inside that window runs its self-cleanup remove_if against a registry that does not hold it yet: the removal finds nothing, the insert then lands a handle for a run that is already over, and nothing ever collects it. state() reports the run as present forever and the registry grows by one every time it happens. The window is short, but the exits that fit inside it are the fast ones - a pre-signalled shutdown, or a goal deleted between the API's read and this call - both of which end the loop before its first agent turn. The handle is now registered before the spawn and the JoinHandle backfilled after. A missing entry at backfill time means the loop already finished and cleaned up, so dropping the handle there is correct: there is nothing left to abort. The generation check keeps a replacement run's handle from being overwritten, and stop() cannot interleave because it takes start_lock, which start() holds for the whole sequence. a_run_that_ends_immediately_leaves_no_entry_behind pre-signals shutdown so the loop breaks on its first check - the shortest path from spawn to remove_if - and runs many rounds on a multi-threaded runtime. It probes the registry directly rather than through state(), which answers None both for "no entry" and for "the state lock was momentarily held" and would otherwise read a transient lock as a clean registry. Hitting the race is probabilistic; the invariant is not. With the handle registered before the spawn there is no ordering in which a finished loop leaves an entry behind, so the test cannot fail spuriously. Pre-existing upstream bug, carried in this PR because it lives in the function the loop-engineering change rewrites. * test(goals): assert on the registry's own agent name AgentEntry carries both `name` (what the registry holds) and `manifest.name` (what the caller asked for). The two diverge when a spawn path renames for uniqueness, and the property under test is what the registry actually ended up holding, so `name` is the field that answers it. `list_arcs` sorts on the same field. * docs(changelog): point the fragment at this PR's number The trailing (#N) group is what the release flow matches against the generated line for this PR, so it has to be #7785. #6505 stays as a mid-bullet cross-reference, which the tooling ignores by design. * fix(dashboard): remove unnecessary escape in goals test * fix(goals): restore goal_run_start signature lost during rebase * fix(goals): sync start_goal_run call with upstream 7-arg signature Move the kernel call after parameter extraction so loop-engineering fields (verify_agent_id, verify_max_retries, evaluator_model) are resolved before they are passed. Update the test call site to match. * fix(goals): resolve all rebase artifacts in loopeng branch Restore kernel_api.rs from origin/main and apply the loop engineering parameter additions on top — the prior rebase replaced the entire impl block with self-recursive default methods, producing 133 compilation errors. Move loop-engineering parameter extraction in routes/goals.rs before the start_goal_run call so the new 7-arg signature is satisfied. * fix(goals): add missing args to goal_runner test call site The start() method takes 11 args after the loop-engineering addition. The handle-unset test was still passing only 5. * fix(goals): update test call sites to match 11-arg start() signature Two tests still used the old 5-arg start() signature after the verifier gate was added. Update both call sites to pass the six new arguments (on_learnings_captured, evaluate_goal, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model) and remove the duplicate test that was added during rebase. * fix(goals): send GOAL_LEARNED lessons to the pending approval queue A goal run wrote its captured lessons straight into the installed skills directory, gated only by the goal's `loop_engineering` flag. That was a second path to skill creation that approved itself, standing next to a workshop (#3328) that deliberately requires an explicit `librefang skill pending approve` before any machine-proposed skill reaches an agent's prompt — and it was reachable by an agent emitting one marker line in its own output. The prompt-injection scan did run, but that is only half the boundary; the other half is that a human has read the thing. The lessons now become a `CandidateSkill` in `pending/<agent>/`, the same queue the background skill reviewer already files into, and only approval installs them. A second run of the same goal files a `CandidateKind::Update` draft when the first run's skill is already installed, so approval routes through `update_skill` rather than failing on the name already existing and dropping what the second run learned. Cap and TTL come from the producing agent's `[skill_workshop]` block, read once at run start. Drafts that did not come from a conversation turn are tagged with a sentinel `explicit_instruction` trigger naming the producer, the convention the reviewer already uses for `auto_evolve_reviewer`; this one is `goal_learned`. Nothing about the durable record changes: the runner still writes `goal_learnings_<id>` to the shared store before any of this, so a draft that is capped out or rejected costs the agent a convenience, not the lessons. * docs(goals): say what an unresolvable evaluator_model does, and give the tick breaker its own entry `verify_agent_id` is validated at save time and `evaluator_model` is not, which reads as an oversight until the asymmetry is written down. A verifier id has a checkable shape and no correct value that fails the check; a model id has neither, and whether one resolves depends on the provider configuration at the moment of the call rather than at save time. So a save-time 400 would reject a model the operator is about to configure and would still not guarantee the id resolves when the run reaches it. The behaviour that is already in place is the right one — a `WARN` per iteration and a fall back to the agent's own marker — and it is now stated on the field and next to the route that stores it. The five-consecutive-failure circuit breaker moves out of the loop-engineering fragment's last line into its own entry, because it is a fix an operator recognises on its own terms: a run pointed at a deleted agent now ends in `Stopped` carrying the error instead of in `MaxIterationsReached` carrying nothing. * fix(kernel,api): close the loop-engineering verifier gate's back doors The #7785 review found that a goal under loop_engineering could still finish on work its own verifier had just rejected, through routes the gate did not close: rejected progress crossed the completion boundary through the pre-existing progress>=100 check, an unreachable verifier fed no circuit breaker and burned the whole iteration budget, and GoalRunner::start's own refusal was discarded and reported as success. Also closes: the completion judge grading a blank statement for a title-only goal, a second run of a goal overwriting the first run's captured lessons, and verify_max_retries/verify_agent_id skipping the boundary validation their sibling fields already have. * fix(kernel,api): close the goal_update tool's verifier-gate bypass /review of 82643bf7f found the progress clamp protected only one of progress's two writers. goal_update is a first-class tool the agent's own system prompt tells it to call, and it patches the same shared document the runner reads independently of parse_tick, so a rejected iteration could still close the run one tick later through the top-of-loop progress>=100 check. The real fix is in that reader: treat bare progress as completion only when no verifier is configured to bypass, which also lets the clamp stop reaching the plain no-verifier and accepted-but-undeclared-done paths it was pinning at 99 for no reason. Also: move the dead-verifier Stopped break after the same per-iteration bookkeeping every other outcome gets, so the run API stops undercounting iterations already paid for; gate the GOAL_BLOCKED exit by the same verified flag GOAL_DONE already uses, closing the one remaining route by which a rejected iteration could end a run; correct a stale comment and an internal-error response that both claimed a 409 which does not exist in this file (the real condition is the goal being gone, i.e. 404); restore a doc comment two of the new #6562 tests had eaten the antecedent of; and fix a changelog fragment whose PR attribution had drifted onto its own line. * fix(kernel): gate the goal_update tool's status field the same as progress Re-review of 1a627f16a found the same tool has a second field that bypassed the verifier gate: goal_update's status="completed" hit the identical top-of-loop GoalStatus::Completed check with no verifier involved, since the field is a fresh enum write untouched by parse_tick. Same fix as progress: bare Completed only ends a verified run through the runner's own done branch now; Cancelled stays unconditioned since it is a legitimate external stop order, not something the gate itself produces. Also: revert the blocked-marker gate from the previous commit — GOAL_BLOCKED is a claim about the agent's own situation, not the task the verifier grades, and gating it left a genuinely blocked agent unable to stop a verified run before burning its whole iteration budget for no completion-safety benefit (blocked never reaches Completed). Rewrite the changelog fragment that still described the superseded write-side clamp instead of the reader-side gate that actually ships. Correct a test doc comment that overclaimed which function the tool's write path shares with the simulation. Pause the clock in three new tests that were sleeping through real TICK_INTERVAL delays. Restore a doc comment line broken across three lines by the previous reflow. * test(kernel): make the status-bypass regression test actually discriminate The prior version passed identically against the commit it was meant to guard against status="completed" bypassing the verifier gate: a successful tick always overwrites goal.status back to InProgress at the end of the same iteration (new_status is never None on that path), so the tool-written Completed never survived to the next header check regardless of whether the header gated on it. The real window is a turn that calls the tool and then fails, since the Err arm never touches the goal document at all. The first tick now fails after writing, which makes the test red against the parent commit's header check and green against this one — confirmed by hand-reverting the header condition, running the test, and reverting back. * fix(goals): give the operator a real stop, and a breaker to every gate leg Six findings from the #7785 re-review. The verifier gate landed in the reader, which closed the `goal_update` tool's bypass but also took the operator's completion path with it — a `PUT` marking a gated goal `completed` no longer stopped the run, and the iteration in flight wrote it back to `in_progress`. Rather than guess which writer produced a stored value, route the operator through the run's real control channel: `update_goal_by_id` now calls `stop_goal_run` the way `delete_goal` always has, and the runner stops writing its own status and progress over a goal whose run has been stopped. The shared top-of-loop condition is unchanged, so #7973 stays converged. The verifier's dispatch errors never reached `classify_tick_error`, so a throttled verifier was reported as a dead one and `RateLimited` never fired for that leg; the rework turn had no breaker at all and could run to the iteration cap. Each leg now carries its own `GateStreak`, split by kind — sharing one counter between legs, or reusing the generator's `error_streak`, produces a counter that is reset or never read. Learnings were appended before the gate and again per rework round with no dedup, so a restated lesson was stored once per round; they now follow the same replace-the-rejected-reply rule as `parsed`, and the run refuses a text it already holds. Both goal endpoints now reject `verify_agent_id == agent_id`, update comparing the effective post-update pair, and the dashboard picker no longer offers the assigned agent. Create drops a blank `evaluator_model` instead of storing "", which is what an earlier review reply claimed it already did. Every test was run against the reverted production block first and recorded failing. * test(api): cover the operator stop at the injection site, not just the helper The kernel test covers the runner's half of the interlock; nothing covered the wiring itself, so deleting `stop_goal_run` from `update_goal_by_id` left the suite green. Four `TestServer` tests: start a run, `PUT` the terminal status, then read back. The follow-up `POST /stop` asserting `stopped: false` is the mechanism-precise part — the registry entry is removed synchronously during the `PUT`, so a second stop finds nothing left. Covers `cancelled` as well as `completed`, and the no-verifier path alongside the gated one that the review reported. The fourth is the negative: an ordinary edit must leave the run alone, which is what stops the interlock from being wired unconditionally. * fix(goals): let a bare operator stop keep the iteration's accounting goal_runner.rs read a plain AtomicBool and skipped patch_goal on any stop, so the two operator paths were treated as one when they are opposites. A terminal PUT /api/goals/{id} writes the document before stopping, and the runner's own end-of-iteration write is exactly what would revert it: new_status is InProgress for every iteration that did not pass verification, and the PUT carries progress too. That write has to be barred. A POST /api/goals/{id}/stop writes nothing. There is no operator choice on the document to protect, so skipping the write only throws away accounting already paid for: the goal keeps the previous iteration's progress while the run row reports the higher iteration count those turns were billed at. Two numbers that contradict each other, with nothing telling the operator which one counts. StopFlag carries who raised it, and stop_after_goal_write is a separate entry point taken only by the terminal PUT. Deletion keeps the plain stop: the rows are already gone, so an in-flight patch_goal updates nothing. wrote_goal is stored before stopped so a thread observing the stop also observes ownership; the reverse order leaves the exact window this distinction exists to close. Verified in red by restoring the previous semantics: the bare-stop test fails with progress 0 against the expected 60, and the terminal-PUT direction alone would have passed either way. * feat(goals): add pause/resume and a configurable loop cadence - Add `Paused` variant to `GoalRunPhase` so a suspended run preserves its iteration count and progress for later resumption. - `tick_interval_secs` on the goal document lets an operator tune the delay between loop iterations (default 2s, clamped 1–86400). - POST /api/goals/{id}/pause — cooperative pause; the in-flight turn finishes, then checkpoints and exits. - POST /api/goals/{id}/resume — continues from the checkpoint; refuses with 409 when there is nothing to resume. - Pause checkpoints are stored in the shared KV store and cleared on resume and on stop, so a stale checkpoint never seeds a fresh start. - Integration tests cover pause-signals-a-live-run, idle-goal-reports- false, and resume-without-pause-is-conflict. Refs #5744 * fix(goals): pass substrate to GoalRunner in tests * fix(goals): resume a paused run under its own cap and timestamps `POST /api/goals/{id}/resume` dispatched with no body, so the kernel substituted `DEFAULT_GOAL_MAX_ITERATIONS` for the cap the paused run was actually under. `GoalRunner::start` seeded the iteration count from the checkpoint while taking the cap from its argument, so a run started with 100 and paused at iteration 30 resumed under a cap of 25 — past the loop's own guard before its first turn, and the exit that follows clears the checkpoint, so the progress the resume was asked to continue was unrecoverable. The cap is now resolved in `GoalRunner::start`, the only layer that knows whether this is a resume: an explicit value wins, then the checkpoint's, then the compiled default. `/resume` accordingly accepts the same optional body `/start` does, and an explicit `max_iterations` there is a deliberate re-budgeting of the remaining run rather than something to ignore. A paused run's `paused_at` was written to the checkpoint and never read back, and `state()` stamped both timestamps with the clock, so two consecutive `GET /api/goals/{id}/run` on a motionless goal disagreed and "how long has this been paused" computed to roughly zero. `ResumePoint` now carries `started_at` and `paused_at`, the checkpoint stores the run's real start time, and both `state()` and `start()` restore them: a resume continues the run it checkpointed rather than opening a new segment, so it keeps the moment it actually began. A checkpoint predating these fields still resumes, falling back to the clock and the default cap. Also drops `goal_tick_sender_context`'s `display_name` parameter, which every caller filled with the same constant the function already hardcodes into `channel`. Tests: five `#[tokio::test]` cases against the real router drive a resume through `POST /resume` from a seeded checkpoint and pin the cap, the override, its validation, and both timestamps; three kernel tests pin the checkpoint round-trip and the cap precedence at the layer that decides it. * feat(goals): gate autonomous runs on a verifier and an evaluator A goal run ended the moment the agent wrote GOAL_DONE, which made the worker the sole judge of its own work - the one check a long-horizon loop most needs, and the one it did not have. Setting loop_engineering on a goal adds two judges that are not the worker. A verifier agent reads each iteration's output and returns VERDICT: PASS|FAIL|NEEDS_REWORK; a rejection goes back to the generator carrying the verifier's stated reason, up to verify_max_retries rework rounds, and until the verifier passes the work GOAL_DONE does not end the run. An evaluator model makes one cheap yes/no read of the goal against the latest output and can conclude the goal is met even when the agent never claimed it. The agent can also record a reusable lesson with GOAL_LEARNED: <text>. Captured lessons are replayed into later iterations' prompts, persisted under goal_learnings_<id>, and written into a goal-learned-* skill through the same prompt-injection scan every other skill-creation path uses. All three are inert unless the goal opts in: with loop_engineering off the prompt is byte-identical to the one it was and the run still makes exactly one LLM call per iteration. a_plain_run_makes_no_extra_llm_calls asserts that, because an evaluator that fires on every goal would double every existing operator's bill silently. Sub-agents are delegated, not provisioned. The prompt directs the agent to its own agent_spawn / agent_send tools, which run under the capabilities its operator granted; neither the runner nor the API ever creates an agent on a caller's behalf. loop_engineering_without_a_verifier_provisions_no_agent and goal_run_start_with_an_unusable_agent_id_provisions_no_agent assert the registry does not grow, which is the property that survives a later refactor - asserting only the status code would not. An unusable verify_agent_id is a 400 with the id named rather than a silently ignored field, because dropping it downgrades a gated run to an ungated one without telling the operator who configured the gate. A run now also gives up after five consecutive non-rate-limit tick failures instead of spending its whole iteration budget rediscovering a deleted agent or a revoked key. Refs #6505 * fix(goals): register a goal run's handle before spawning its loop start() spawned the loop task and inserted its RunHandle afterwards. A loop that finishes inside that window runs its self-cleanup remove_if against a registry that does not hold it yet: the removal finds nothing, the insert then lands a handle for a run that is already over, and nothing ever collects it. state() reports the run as present forever and the registry grows by one every time it happens. The window is short, but the exits that fit inside it are the fast ones - a pre-signalled shutdown, or a goal deleted between the API's read and this call - both of which end the loop before its first agent turn. The handle is now registered before the spawn and the JoinHandle backfilled after. A missing entry at backfill time means the loop already finished and cleaned up, so dropping the handle there is correct: there is nothing left to abort. The generation check keeps a replacement run's handle from being overwritten, and stop() cannot interleave because it takes start_lock, which start() holds for the whole sequence. a_run_that_ends_immediately_leaves_no_entry_behind pre-signals shutdown so the loop breaks on its first check - the shortest path from spawn to remove_if - and runs many rounds on a multi-threaded runtime. It probes the registry directly rather than through state(), which answers None both for "no entry" and for "the state lock was momentarily held" and would otherwise read a transient lock as a clean registry. Hitting the race is probabilistic; the invariant is not. With the handle registered before the spawn there is no ordering in which a finished loop leaves an entry behind, so the test cannot fail spuriously. Pre-existing upstream bug, carried in this PR because it lives in the function the loop-engineering change rewrites. * test(goals): assert on the registry's own agent name AgentEntry carries both `name` (what the registry holds) and `manifest.name` (what the caller asked for). The two diverge when a spawn path renames for uniqueness, and the property under test is what the registry actually ended up holding, so `name` is the field that answers it. `list_arcs` sorts on the same field. * docs(changelog): point the fragment at this PR's number The trailing (#N) group is what the release flow matches against the generated line for this PR, so it has to be #7785. #6505 stays as a mid-bullet cross-reference, which the tooling ignores by design. * fix(dashboard): remove unnecessary escape in goals test * fix(goals): restore goal_run_start signature lost during rebase * fix(goals): sync start_goal_run call with upstream 7-arg signature Move the kernel call after parameter extraction so loop-engineering fields (verify_agent_id, verify_max_retries, evaluator_model) are resolved before they are passed. Update the test call site to match. * fix(goals): resolve all rebase artifacts in loopeng branch Restore kernel_api.rs from origin/main and apply the loop engineering parameter additions on top — the prior rebase replaced the entire impl block with self-recursive default methods, producing 133 compilation errors. Move loop-engineering parameter extraction in routes/goals.rs before the start_goal_run call so the new 7-arg signature is satisfied. * fix(goals): add missing args to goal_runner test call site The start() method takes 11 args after the loop-engineering addition. The handle-unset test was still passing only 5. * fix(goals): update test call sites to match 11-arg start() signature Two tests still used the old 5-arg start() signature after the verifier gate was added. Update both call sites to pass the six new arguments (on_learnings_captured, evaluate_goal, loop_engineering, verify_agent_id, verify_max_retries, evaluator_model) and remove the duplicate test that was added during rebase. * fix(goals): send GOAL_LEARNED lessons to the pending approval queue A goal run wrote its captured lessons straight into the installed skills directory, gated only by the goal's `loop_engineering` flag. That was a second path to skill creation that approved itself, standing next to a workshop (#3328) that deliberately requires an explicit `librefang skill pending approve` before any machine-proposed skill reaches an agent's prompt — and it was reachable by an agent emitting one marker line in its own output. The prompt-injection scan did run, but that is only half the boundary; the other half is that a human has read the thing. The lessons now become a `CandidateSkill` in `pending/<agent>/`, the same queue the background skill reviewer already files into, and only approval installs them. A second run of the same goal files a `CandidateKind::Update` draft when the first run's skill is already installed, so approval routes through `update_skill` rather than failing on the name already existing and dropping what the second run learned. Cap and TTL come from the producing agent's `[skill_workshop]` block, read once at run start. Drafts that did not come from a conversation turn are tagged with a sentinel `explicit_instruction` trigger naming the producer, the convention the reviewer already uses for `auto_evolve_reviewer`; this one is `goal_learned`. Nothing about the durable record changes: the runner still writes `goal_learnings_<id>` to the shared store before any of this, so a draft that is capped out or rejected costs the agent a convenience, not the lessons. * docs(goals): say what an unresolvable evaluator_model does, and give the tick breaker its own entry `verify_agent_id` is validated at save time and `evaluator_model` is not, which reads as an oversight until the asymmetry is written down. A verifier id has a checkable shape and no correct value that fails the check; a model id has neither, and whether one resolves depends on the provider configuration at the moment of the call rather than at save time. So a save-time 400 would reject a model the operator is about to configure and would still not guarantee the id resolves when the run reaches it. The behaviour that is already in place is the right one — a `WARN` per iteration and a fall back to the agent's own marker — and it is now stated on the field and next to the route that stores it. The five-consecutive-failure circuit breaker moves out of the loop-engineering fragment's last line into its own entry, because it is a fix an operator recognises on its own terms: a run pointed at a deleted agent now ends in `Stopped` carrying the error instead of in `MaxIterationsReached` carrying nothing. * fix(dashboard): the run-phase union and the badge must know about "paused" GoalRunState.phase is "typed from the wire contract rather than string so a caller cannot pass a typo'd literal" — but this PR's own server now sends "paused", a value the type said could not exist. The badge then fell through to the default styling and GoalsPage rendered a bare untranslated "paused" in every language, while every other phase had a locale key. The union gains "paused", the badge gains a case (warning palette — the run is alive but not progressing, same family as stopped), and goals.run_phase_paused lands in all five locales. The pause/resume dashboard client (pauseGoalRun / resumeGoalRun wrappers and their mutation hooks) is deliberately not here: it arrives in #8029, which is stacked on this branch and adds the controls that use it. * fix(kernel): goal runner resume, verifier gate, and pause correctness Four defects the maintainer review of #7973 found unaddressed after the review pass: - run_loop counted iterations from a hardcoded 0 instead of the checkpointed value start() already resolved into the run's state, so a resumed run got a fresh iteration budget rather than continuing the one it was paused under. - A verifier-rejected iteration's unclamped GOAL_PROGRESS: 100 let the next iteration's plain progress>=100 check finish the run anyway, bypassing the verifier gate entirely. - Pause/stop were only observed at the top of the outer loop, so a tick_interval_secs near its 24h ceiling left a pause request unobserved for up to a day; the inter-tick sleep now polls every second instead of sleeping the whole interval in one shot. - Pausing discarded whatever GOAL_LEARNED: lessons the run had captured so far. The resume checkpoint now carries them and threads them back into the resumed run's own accumulator. * fix(kernel): goal run learnings respect the skill workshop opt-in queue_learnings_as_pending_skill was called unconditionally from the goal runner's on_learnings hook, so a goal run queued a pending skill draft even for agents that never set [skill_workshop] enabled = true in agent.toml. Nothing else in the goal-run path consulted the workshop config -- only the approval-side CLI/API/dashboard did. Extracted the gate into should_queue_learnings (enabled && auto_capture), matching the check every other automatic capture path in the workshop already applies. * fix(api): validate verify_max_retries and blank verifier/evaluator fields start_or_resume read verify_max_retries with a bare .map(|n| n as u32), so an out-of-range or wrongly-typed value silently wrapped instead of 400ing -- the same field max_iterations already validates properly a few lines above. create_goal's verify_agent_id parsing didn't go through optional_uuid_field, so a blank string (the create form's clear signal, per #6562) 400ed as "Invalid verify_agent_id" instead of being treated as unset. evaluator_model_str had no blank filter either, so a blank string was stored verbatim as Some(""), contrary to the field's own documented None-means-unconfigured contract. * fix(cli): goal --watch recognizes the paused run phase terminal_phase_message had no branch for "paused", so a goal that transitioned into paused mid-watch (an operator pausing it from the dashboard, say) read as an unclassified state: classify_poll routed it to Unobservable, burning the bounded retry budget before --watch gave up with a generic "outcome unknown" message instead of reporting that the run was paused. Added the branch plus a locale key in all four CLI locales, and extended every_terminal_phase_has_a_summary to cover it. * fix(kernel): close the goal_update verifier-gate bypass and harden pause/resume /review of 56e75aee0..562f414b5 found the progress clamp from the first pass protected only one of goal.progress's two writers: goal_update (the tool the agent's own system prompt tells it to call) patches the same shared document directly, bypassing parse_tick entirely, so a rejected iteration could still close the run one tick later through the bare top-of-loop progress>=100 / status==Completed check. PR #7785 found and fixed the identical bug first; adopted its reader-side gate verbatim (bare progress/status only count as done when no verifier is configured; Cancelled stays unconditioned) so the two PRs converge instead of one reverting the other on merge, and kept the clamp as defense in depth scoped to !verified. Also from the same review: - The inter-tick sleep now wakes to a fixed deadline via sleep_until instead of chaining PAUSE_POLL_INTERVAL sleeps end-to-end, which would have drifted by scheduling latency accumulated once per slice (on the order of a minute over the 24h maximum tick interval). - verify_max_retries now survives a pause the same way max_iterations already does: added to the resume checkpoint with the same argument-then-checkpoint-then-default precedence. - run_loop_resumes_from_the_checkpointed_iteration_not_zero passed identically with and without the iteration-reset fix it was meant to guard, because the final iteration count at MaxIterationsReached is the same either way. Now counts turns and inspects the first resumed prompt, which does discriminate. - learnings_survive_a_pause_and_resume_cycle asserted an exact ordered list that only held if exactly one tick landed before pause() took effect; a second tick under load duplicated a fixed lesson. The send closure now returns a distinct lesson per turn and the assertion checks membership/uniqueness instead of an exact list. - Added a regression test for pause and shutdown signalled together mid-sleep: both now resolve through the same pause-before-shutdown check at the top of the loop, so they land on Paused rather than the old flat select! hardcoding Stopped in that one path. * fix(kernel): move the skill-workshop opt-in gate into its single callee /review of 83e9bfecf found should_queue_learnings gated only the caller (goal_run_start's on_learnings closure), leaving queue_learnings_as_pending_skill's own two pre-existing tests untested for the opt-in -- both called it directly with the disabled default and asserted a draft got queued, which is exactly what the fix was meant to stop. Moved the gate inside the function itself (its only production caller), updated those two tests to enabled: true since they were asserting the pre-fix behavior, and added a test asserting a disabled workshop queues nothing when called directly. * fix(api): refuse a resume cap at or below the checkpoint's iteration /review of 56e75aee0 found a side effect of the iteration-reset fix: GoalRunner::start now compares an explicit max_iterations against the RESTORED iteration count instead of 0, so a resume body naming a cap at or below that count (e.g. {"max_iterations": 10} on a run paused at iteration 30) trips the cap check on the very first pass with no turn run, and clears the checkpoint -- and the learnings it carries -- on the way out, for a request that could never have advanced the run. start_or_resume now reads the goal's run state once (reused for the existing require_paused check) and rejects such a cap with a 400 naming both numbers, on both /start and /resume since /start also auto-resumes from an existing checkpoint. * fix(kernel): correct stale doc comments and cover the registry-absent case /review of 378b8bfa4 found two loose ends: - The doc comments on GoalRunner::start and its test described an explicit resume max_iterations as "re-budgeting the remaining run", which is not what the code does or what the earlier fix in this branch made it do: the cap is a TOTAL ceiling compared against the restored iteration count, exactly like a fresh start compares against 0. Corrected both, and noted the new below-checkpoint 400 in the API route's own doc comment. - should_queue_learnings' own tests only exercised it as a pure predicate; nothing verified the other half of goal_run_start's wiring, that an agent absent from the registry (deleted, never spawned, mistyped id) still resolves to a denying config rather than some other default. Extracted that resolution into resolve_workshop_config so it's callable without a live turn, and added two tests: absent-agent denies, and a registered agent's own manifest setting is what actually gets read (proving the first test isn't passing because the function ignores the registry outright). Verified the new test actually guards something: temporarily made the fallback permissive (enabled: true instead of ::default()) and confirmed resolve_workshop_config_denies_when_the_agent_is_absent_from_the_registry fails before restoring it. * fix(kernel): cancel discards a pause checkpoint it cannot read `load_pause_checkpoint` answers `None` for two different things: there is no checkpoint, and there is a row it could not turn into a `ResumePoint`. A substra…

github-actions Bot added size/L 250-999 lines changed area/kernel Core kernel (scheduling, RBAC, workflows) labels Aug 29, 2026

DaBlitzStein force-pushed the feat/goal-pause-resume branch from 855a9be to 1662239 Compare August 29, 2026 15:09

houko reviewed Aug 30, 2026

View reviewed changes

houko mentioned this pull request Aug 30, 2026

feat(cli): add the goal command and a Goals tab to the TUI #7784

Merged

DaBlitzStein mentioned this pull request Aug 30, 2026

feat(dashboard): add pause/resume controls to the goals page #8029

Merged

DaBlitzStein force-pushed the feat/goal-pause-resume branch from f565542 to 1662239 Compare August 30, 2026 18:42

houko mentioned this pull request Sep 1, 2026

feat(dashboard): display goal run phase badges #8067

Merged

houko previously requested changes Sep 1, 2026

View reviewed changes

houko reviewed Sep 1, 2026

View reviewed changes

houko requested changes Sep 2, 2026

View reviewed changes

github-actions Bot added the needs-changes Changes requested by reviewer label Sep 2, 2026

DaBlitzStein mentioned this pull request Sep 3, 2026

feat(goals): gate autonomous goal runs on a verifier and an evaluator #7785

Merged

github-actions Bot added size/XL 1000+ lines changed and removed size/L 250-999 lines changed labels Sep 5, 2026

github-actions Bot added the area/docs Documentation and guides label Sep 7, 2026

github-actions Bot added ready-for-review PR is ready for maintainer review and removed needs-changes Changes requested by reviewer has-conflicts PR has merge conflicts that need resolution labels Sep 13, 2026

DaBlitzStein mentioned this pull request Sep 13, 2026

feat(tui): configure and show loop engineering on the goals screen #8229

Closed

DaBlitzStein and others added 3 commits September 13, 2026 23:34

Merge branch 'main' into feat/goal-pause-resume

feaea20

Merge branch 'main' into feat/goal-pause-resume

8258764

github-actions Bot removed the area/docs Documentation and guides label Sep 14, 2026

houko merged commit 1eb96d8 into librefang:main Sep 14, 2026
46 checks passed

DaBlitzStein added a commit to DaBlitzStein/librefang that referenced this pull request Sep 14, 2026

merge: librefang#7973 feat/goal-pause-resume

dcb69dd

DaBlitzStein mentioned this pull request Sep 14, 2026

fix(kernel): restore checkpoint verify_max_retries on paused goal readout #8359

Merged

houko mentioned this pull request Sep 19, 2026

release: v2026.9.19 #8436

Merged

Conversation

DaBlitzStein commented Aug 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Out of scope

Test plan

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Blocking

Coordination

Non-blocking

Uh oh!

houko left a comment

Choose a reason for hiding this comment

Uh oh!

houko left a comment

Choose a reason for hiding this comment

1. POST /api/goals/{id}/resume replaces the run's iteration cap with the default, and can terminate the resumed run immediately

2. A paused run's started_at / updated_at are minted fresh on every poll; the persisted paused_at is never read

3. No changelog fragment

4. Nit: goal_tick_sender_context's display_name parameter

Verified separately

Uh oh!

DaBlitzStein commented Sep 3, 2026

Uh oh!

DaBlitzStein commented Sep 3, 2026

Uh oh!

DaBlitzStein commented Sep 4, 2026

Uh oh!

DaBlitzStein commented Sep 7, 2026

Uh oh!

DaBlitzStein commented Sep 7, 2026

Uh oh!

DaBlitzStein commented Sep 7, 2026

Uh oh!

DaBlitzStein commented Sep 7, 2026

Uh oh!

DaBlitzStein commented Sep 7, 2026

Uh oh!

DaBlitzStein commented Sep 7, 2026

Uh oh!

DaBlitzStein commented Sep 13, 2026

Uh oh!

DaBlitzStein commented Sep 13, 2026

Ahora se apila sobre #7785 (2adb4cba4)

Uh oh!

DaBlitzStein commented Sep 13, 2026

Esta rama no compilaba: arreglado en 8fbb34da5

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DaBlitzStein commented Aug 29, 2026 •

edited

Loading

1. `POST /api/goals/{id}/resume` replaces the run's iteration cap with the default, and can terminate the resumed run immediately

2. A paused run's `started_at` / `updated_at` are minted fresh on every poll; the persisted `paused_at` is never read

4. Nit: `goal_tick_sender_context`'s `display_name` parameter

Ahora se apila sobre #7785 (`2adb4cba4`)

Esta rama no compilaba: arreglado en `8fbb34da5`