Repository navigation
Batch G: runon re-attach, bash timeout task ids, scratch retention, health and test fixes - #623
Merged
Merged
Conversation
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 8532116815bb4dec84260db4dee5dd997df845b5)
When a standalone bash/powershell call outlasted the bridge's per-call deadline, the bridge deleted the pending entry and rejected at once, and adaptToolError appended "no background task was created for this command and no task ID exists. Re-run the command." AFT, however, had usually received the request, started the command and answered a little later with its task_id; that reply was silently dropped. The agent got a false "no task" claim for a command that was running, and re-running it duplicated the work. Mechanism: a short late-reply grace window in the standalone bridge (option (a)). For a keepBridgeOnTimeout bash/powershell call (both hosts always send bash this way), the deadline no longer rejects immediately: the pending entry stays for up to 10 s (BridgeOptions.lateShellReplyGraceMs) and the call then rejects with BridgeTransportTimeoutError carrying a shellOutcome built from what AFT actually said: - late reply with task_id -> "AFT started this command as task <id> ... Use bash_status or bash_watch with task ID <id> to follow it; do not re-run the command." - late error reply without task_id -> AFT's own code/message and "no task ID was assigned". - no reply in the window -> "whether this command started is unknown. The command may not have started; check bash_status before rerunning." The ordinary reply path still records the task for the event-loop hold. A reply that arrives after the window is now logged (with its task id) instead of dropped silently. Why not option (b), asking the engine by correlation id: the engine does not record the request id at spawn (tasks carry a separate call_key, and bash_owned_tasks lists a session's task ids with no request key), so it would need new engine state plus a new query command; and the query would travel over the same busy engine that made the original call late, so it would often time out too. The grace window reports exactly the engine's own reply and needs no engine change. adaptToolError no longer treats a bridge timeout as a "transport dead" bash failure: the re-run disposition stays only for failures proven to happen before dispatch. The existing test "adds bash transport disposition guidance while preserving the error" asserted the false "no task was created" claim for a bridge timeout; it is replaced by tests asserting the unknown/task/refusal dispositions, because a timed-out request was sent and may have started. The subc transport does not produce this message (its request deadline error carries code request_deadline and gets no bash disposition), so it is unchanged. Both hosts format the error only through adaptToolError, so the fix in aft-bridge covers OpenCode and Pi; each gets a bash-tool test. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 5076e85cb7d1270d3ab7abd549d4b868f7648748)
…ter 5 minutes When ck-motor restarts or its route closes mid-job, AFT now keeps re-attaching (never resubmitting) with exponential backoff (100ms doubling to 30s) until the job's own limits have passed: ck-motor's default queue wait limit (300s) plus the command's run timeout, counted from submission, plus 5 minutes of grace, and never sooner than 5 minutes after recovery began. A terminal record (including ck-motor's tombstone for an unknown job or history_expired) still ends it at once. The give-up message names its cause and the limits. The unknown-outcome advice no longer points at `exec.status <id>`, which no agent tool can call: it says the job may still hold the workspace for up to its run timeout and to wait, then rerun. Internal recovery texts that named exec.status are reworded too. A queue_wait_exceeded refusal whose ck-motor detail says `waiting_on=workspace owner` and lists, among its holders, this session's own earlier job in the same checkout now says "your earlier job <job> in this workspace is still running (task <task>)". The holder is persisted in the remote record so restarted readers render the same text. Test changes: assertions on the old "could not be re-attached for 5 minutes" and "check exec.status" texts were updated to the new wording because both texts are removed on purpose (the 5-minute cap is gone and exec.status is not an agent tool); the properties they guard (named job, command not rerun, rerun-safety advice) are still asserted. The subc format parity fixtures carrying the old advice text were updated to the new text. New tests: an outage fake (ck-motor provider absent from the catalog, attach replays from from_seq without `accepted`, all frames from the published exec-remote types) keeps the route down past six minutes on an injected clock and the result still arrives; the same outage past the job's own limits reports outcome unknown with the named cause. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 877da1c26d563249fdfcdf5e05ef2e226c5b94df)
A runon call without a timeout gets HardKill::Default, so spawn_remote records and sends the 30-minute background default as the job's run timeout. Add a test for that call that survives a 25-minute outage, and assume the same default for a record with no recorded timeout instead of falling back to the bare 5-minute grace. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit b8d59268921eacc5a4a7ab4e0c3fab5a627d10f8)
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 744f653b3142b8db17bc7cf4a9d3f2e993f86e76)
Health rollup: each resident index's memory walk is now reused by its own shape, so a symbol-cache change (every read) re-walks only the symbol cache instead of the trigram and semantic indexes too. Thread names are classified at most every 30 s in the rollup (total still counted every pass) and the census reports threads.classified_age_ms. The offline rollup bench gains knobs for roots, idle threads, per-root index size and churn, and builds on macOS as well as Linux. Search index: SearchIndex::apply_search_update refuses paths that the disk cache could not store under the root (storable_relative_path, the same rule the cache writer uses). AFT's own writes reported every touched path, including edits in sibling checkouts and temp dirs, and one such entry made every later cache write fail with 'refusing to cache path outside project root'. Symbol cache: the writer now persists the entries under its root and skips parsed files outside it, instead of refusing the whole save. The test symbol_cache_rejects_paths_outside_project_root_on_write becomes symbol_cache_persists_only_paths_under_project_root: the property it protected (no outside path is ever written) still holds and is asserted on reload; what changes is that in-root entries are saved alongside. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 5aa5b4e8e9089e637185c5b54d8e4807e83b5cad)
…reads it The Windows test build has no reader for thread_classifications_for_test, so -D warnings rejected it as dead code. Rollup bench (linux, 4 vCPU, CPU us per tick; 120 roots, 300 extra threads, 2000 files per root, churn on): per-root memory census 22258 -> 4781, lifecycle OS probes 5313 -> 1173, total 58268 -> 26979. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 2b56b5fa444d19227a719ed32a2fe4a7e0faa2f5)
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…eep reads A configured BgTaskRegistry writes task JSON while it has no aft.db handle: configure opens aft.db after the bind is acknowledged, and idle reap clears the handle on unbound roots. Each such write logged a WARN and its row was lost; set_db_pool only rewrote missing rows of still-running tasks, so a task that finished in the gap kept a missing or stale 'running' row. Skipped tasks are now recorded and set_db_pool writes their current rows; the gap is logged once per interval at INFO instead of a WARN per write. cleanup_url_cache ran on every configure and opened and parsed every url_cache/*.meta.json. Unchanged metadata files are now recognized by size and mtime from an in-memory memo and cost one stat. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 527d490033fc5ebf2f5807c686e56cb1818aac18)
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 8e2ed3755e0fd105354abcaadc024741dfb4f658)
Diffs and logs written to /tmp or TMPDIR raise a read-permission ask for the head every time a worker reads them back. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
A native delete_file on a loaded Windows CI runner failed with
read_io_queue_timeout ("filesystem helper did not start before the
deadline; this call did not start"). The failing call was the path
check in AppContext::check_path_access, reached from
validate_write_location: bounded_io::run spawns a fresh helper thread
on the shared 64-slot limiter, and run_with_timeout capped the wait for
that thread to start at min(request deadline, queued + 5 s). A native
request has no caller budget, so validate_write_location created a 5 s
one and the queue cap equalled it. The log shows the helper was not
scheduled for the whole window: the delete began right after 47.94 s
and failed about 5 s later, exactly while another thread was loading
the ONNX Runtime DLL (47.988 s to 54.179 s); on Windows a new thread
cannot start while the loader lock is held. Edit, write, apply_patch,
move and batch use the same validators and had the same exposure; no
mutation path had a different pattern.
A helper that has not started has touched nothing, and a mutation has
no partial answer, so refusing is the wrong outcome. Mutating validators
(validate_path, validate_write_location) now enter a MutationScope: a
25 s request budget when none exists (the budget agent tool calls
already run under), and helper admission may wait until that request
deadline instead of one access cap. The access cap after the helper
starts is unchanged, and the owner now re-checks once per access cap
while waiting, so a helper that starts late and then hangs still fails
as read_blocked at its own access deadline. Reads keep the old queue
cap. A start that misses the request deadline still reports
read_io_queue_timeout with the existing "this call did not start" text.
Tests (unix seams that hold a helper unscheduled): tool-call-shaped and
native deletes succeed when the helper starts after the access cap (both
red before the fix with the CI error text), a started-then-hung helper
under a mutation still fails read_blocked quickly, and reads still get
read_io_queue_timeout.
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
(cherry picked from commit e7fb2f16b0c42e556a52f47ea377b3d34e237c16)
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…vels A call-graph answer captures what is pending before it is computed. A changed file moves through the watcher's flush window, then the dispatch channel and the watcher drain, then the legacy store's refresh worker (queued, active, waiting), and is either committed or parked in the pending sink. Other threads move it while the capture reads, so a read order that goes against that travel can miss it, and the answer then shows the old edge as complete: - dispatch read the dispatch channel and drain (watcher_query_pending) before the flush-window set (callgraph_query_pending), so a flush between the two reads hid the file from both; - the pending sink was read before the worker queue, so a batch that parked its files and left the worker between the two reads hid them from both. Dispatch (main.rs and run_tool_call.rs) now captures both values in one pass, response_finalize::query_pending through AppContext::pending_changes_for_query. The pass reads, in order: 1. the flush-window set; 2. the dispatch channel, drain slot and drains-in-flight count, between two reads of a new drain-activity counter. A drain advances the counter when it starts and when it ends, so a drain that moves work during these reads is caught. The slot and the in-flight count hand work to each other in both directions, so read order alone cannot cover that hand-off; 3. the pending sink, then the worker queue, then the sink again, because files move both ways between those two; 4. with views on, the drain's publication paths, then the publication jobs. Producer write order was checked for every hand-off. Each must write the next stage before it clears the previous one. The watcher thread, the drain and the worker's defer already did. Replaying the sink into the worker did not: it cleared the sink and only then enqueued, in drain_callgraph_store_events, in the inline store install in callgraph_store_for_ops, and in RefreshBatch::defer_after_open_failure. All three now enqueue first and then remove the replayed paths from the sink. If the enqueue is refused, the replayable paths stay parked for the next ready store; before this change the unbound case lost them. Deterministic lib tests use a cfg(test) hook that runs between two stage reads and moves a file one stage downstream: - a_flush_between_two_capture_reads_still_names_the_file (flush-window set to dispatch channel); - a_refresh_parked_between_two_capture_reads_still_names_the_file (worker batch to pending sink). Each goes red when the old read order is restored. callgraph_query_pending and AppContext::callgraph_unapplied_changes are replaced by query_pending and pending_changes_for_query, which had no other callers. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com> (cherry picked from commit 58da24b4eb495760add5a0471ec63d968ac7aae3)
Release builds warned about an unused import because the only user is the debug_assertions path list. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…30 minutes A sandboxed command's $TMPDIR (/private/tmp/aft-w/<id> on macOS, /tmp/aft-w/<id> on Linux) was deleted the moment the command exited, so a file written there in one shell call could not be read with AFT's read tools in the next step. Terminal cleanup now keeps the directory until exit + SCRATCH_RETENTION (30 minutes). The delete time is persisted in the task record (sandbox_temp_delete_at; older records derive it from finished_at), and metadata.sandbox_temp_dir stays set meanwhile, so the existing own-scratch read exemption applies and other sessions, writes and symlink escapes keep their policy. A kept task leaves the watchdog and is listed in the registry; the once-a-minute cleanup lane hands due entries back to the watchdog, which deletes them through the existing scratch cleanup worker. Replay after a restart deletes scratch whose time has passed and keeps the rest. A session scope end and pruning of the task record delete it early. The health snapshot reports retained scratch count and a bounded, sweep-measured byte total. Existing scratch-cleanup tests now set a past delete time or move the clock past it, since they test the deletion machinery rather than the exit timing; the not-found test for removed scratch does the same. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…lease precisely Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…se note Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Replaying parked call-graph paths into the refresh worker enqueued them and then removed them from the pending sink whenever the enqueue returned true. The worker also returns true when it merges the paths into a batch that keeps waiting for another process's writer. That batch is retired when the root unbinds, on the assumption that its paths are still parked, so the edits were in neither place and the next bind could not replay them. The worker now takes paths out of the sink itself, under its queue lock, at the moment it queues them for work. A batch that keeps waiting leaves them parked, and callers that replay parked paths no longer remove the replayed ones (settle_replayed_callgraph_store_paths only drops the parked paths that were not replayed). RefreshBatch::defer_after_open_failure had the same remove-after-enqueue step; a shutdown that gave up on the queued replay between the enqueue and that removal lost the paths. It now relies on the worker too. The upstream-first read order of the pending-change capture is unchanged. A root whose call-graph index is disabled or degraded keeps replayed paths parked instead of dropping them, since no worker takes them. Tests: - an_edit_replayed_into_a_batch_waiting_for_a_busy_writer_survives_an_unbind (context.rs): busy writer keeps the batch waiting, replay, unbind retires the batch, the answer still names the edit, and the next bind hands it to the worker again. Red when settle removes replayed paths again. - open_failure_replay_given_up_by_shutdown_stays_parked (callgraph_store): red when the post-enqueue removal is restored. New test seams: writer_busy and open_failure_replay_hold. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…anup
A command can finish just before its session scope ends and reach scratch
cleanup only afterwards. The scope-end pass (kill_scope, then
release_scope_scratch) only visits scratch already listed as kept, so it
missed that command, and cleanup then kept its folder for the full 30-minute
retention.
cleanup_terminal_task_temp_at now lists the kept scratch first and then reads
the same ended-scope record kill_scope writes (revoked_scopes). If the
command's scope has ended, it marks the scratch released and schedules the
deletion at once. kill_scope records the end before it reads the kept list,
so whichever side runs second sees the other and the scratch is released in
every order. kill_scope_if_revoked shares the new scope_revoked lookup.
Test: bash_background_scratch_retention_scope_ended_before_cleanup_deletes_early
ends the scope before either finished command reaches cleanup; the ended
scope's scratch must be deleted and the other scope's kept. Red ("scratch of
a scope that ended before cleanup was kept") with the ended-scope check
disabled; the other scratch-retention tests stay green.
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
login_shell_probe_survives_concurrent_fork_stress relied on the parent's spawn() landing inside the forked child's 15 ms sleep. A paused runner could let the child exec and drop the writable descriptor first, so expect_err failed although the retry was correct. The child now blocks reading a pipe while it holds its copy of the writable descriptor. After the first busy-file assertion, the probe runs with a test-only hook (busy_retry_hook, called by spawn_retrying_executable_file_busy before each busy retry, per thread). On the first retry the hook releases the child and waits on a second pipe until the child has closed the descriptor, so the retry that follows must succeed. No wall-clock window remains, and the test now also asserts that the probe really retried. Red with SPAWN_BUSY_RETRY_COUNT = 0: "the probe's first spawn must find the file busy and retry". Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
…sion
ReattachBudget fixed the job's end from the task's first submission
(started_at). A draining retry resubmits the job, and the runner's queue wait
and run timeout start again from that resubmission. After nine minutes of
maintenance an untimed command could still have most of its 30-minute run
ahead, while AFT's recovery expired about 40 minutes after the original
submission and gave up on a job that was still running.
submit_draining_retry now records the resubmission time in the task record
(DrainingRetry.resubmitted_at_ms, written with the dispatch). ReattachBudget
counts from it, falling back to started_at for records without it, and the
worker rebuilds its limit (ReattachBudget::resubmitted) after each draining
resubmission, including one replayed after a restart.
Tests:
- reattach_limit_counts_from_a_late_draining_resubmission: a resubmission
nine minutes after the first submission gets the full 35-minute window in
the live worker and after the record is written and read back; a record
without the field counts from the start. Red ("26.00 minutes left") when
the resubmission time is ignored.
- runon_draining_twice_then_accepts_and_discloses_wait now asserts the
accepted resubmission's time is recorded. Red when the write is removed.
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
A finished command's scratch now stays for its own session to read, so the jail test no longer expects it deleted at once. It waits for the first command to finish, then checks that the retained scratch is still unwritable by another task and its sentinel unchanged. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
jacobocasado
pushed a commit
to starredjaco/aft
that referenced
this pull request
Oct 20, 2026
Rebase the release prep onto origin/main 3faaded and extend .alfonso/release-notes/v0.61.0.md to cover the merges after cortexkit#590: cortexkit#578, cortexkit#596, cortexkit#600, cortexkit#601, cortexkit#612, cortexkit#613, cortexkit#614, cortexkit#616, cortexkit#617, cortexkit#618, cortexkit#619, cortexkit#620, cortexkit#622 and cortexkit#623. Fold and delete the 27 next/ fragments those merges added. Credit cortexkit#465 (@null-axiom) and cortexkit#564 (@iceteaSA), and extend the cortexkit#577 line with the shell timeout task-id fix. Refresh ANNOUNCEMENT_FEATURES in both plugins. Pi leaves out the OpenCode 2 permission line, which does not apply to Pi users. Co-authored-by: alfonso-aft <289616620+alfonso-aft@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ten settled fixes, each reviewed and tested on its own:
Summary by cubic
Ten settled fixes to bash/runon reliability, health-check cost, and several smaller correctness issues, each reviewed and tested on its own.
runonre-attaches to a live remote job for the runner's queue wait plus the job timeout plus 5 minutes instead of giving up after 5 minutes; the window counts from the latest draining resubmission, which restarts the runner's wait and job timeout.bash_statusandbash_watchcan follow it (Standalone status refresh repeatedly waits for inspect writer lease, blocking unrelated tool requests #577).aft.dbfinishes opening are saved once it opens.Written for commit 5839009. Summary will update on new commits.
The PR appears safe to merge; no actionable issues remain from this review.Summary
This PR improves shell task recovery, scratch retention, search cache saves, and health-check cost.
All four earlier findings are addressed in the current code. No new actionable issues were found.
Reviews (3) · Last reviewed commit: "Check a finished command's scratch again..." · Reviewed by Greptile