docs(#7042): say the budget kill sweeps, best effort, and why - #7064
docs(#7042): say the budget kill sweeps, best effort, and why#7064waynesun09 wants to merge 1 commit into
Conversation
The budget-and-deadline docs said the runner "terminates the agent's processes" at the budget. What the code does is end the exec and run the stray-process sweep, which is best effort by its own contract: OpenShell has no per-exec kill by design, an exec's processes are not expected to exit with the caller (NVIDIA/OpenShell#3159, closed as expected usage, not as a refused feature). Align run.md and the harness reference with the rule bullet that already said so, and add a short note to ADR 0105 connecting its #3159 citation to that reading. The console blocks are real output and stay as pasted. Assisted-by: Claude Signed-off-by: Wayne Sun <gsun@redhat.com>
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all |
PR Summary by QodoClarify best-effort timeout process cleanup
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
|
🤖 Finished Review · ✅ Success · Started 4:07 PM UTC · Completed 4:20 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.88 |
Site previewPreview: https://ec603eff-site.fullsend-ai.workers.dev Commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Risk Assessment: moderate (2/5) DetailsPure documentation PR touching 3 actively-maintained files with 22 lines changed, clear issue linkage, no security or CI risk — routine but warrants a standard review pass due to active churn on the affected docs. |
ReviewFindingsLow
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| Note (2026-09-06): [NVIDIA/OpenShell#3159](https://github.com/NVIDIA/OpenShell/issues/3159) | ||
| was closed as expected usage rather than a refused feature: an exec's processes | ||
| are not expected to exit with the caller, and a runner that reuses one sandbox | ||
| across execs owns their cleanup. The sweep above is that cleanup, and it is |
There was a problem hiding this comment.
[low] internal-consistency
The appended Note says 'The sweep above is that cleanup, and it is best effort', but the Decision section at line 51 says 'The runner first terminates the agent's processes in the sandbox.' The Note's referent is ambiguous and the terminology is inconsistent within the ADR. Acknowledged as intentional (Decision text untouched per ADR immutability), but may confuse readers.
| every iteration it tells the agent when that will happen, through two environment variables set on | ||
| every runtime (claude, pi, codex): | ||
| is spent the runner ends the iteration and sweeps the processes the agent left running in the | ||
| sandbox, best effort. Before every iteration it tells the agent when that will happen, through two |
There was a problem hiding this comment.
[low] naming-convention
The qualifier 'best effort' is formatted inconsistently: line 129 uses comma form ('sandbox, best effort.') while line 181 and harness-reference.md line 146 use parenthetical form ('sandbox (best effort)'). The comma form is defensible at end-of-sentence but adopting parenthetical everywhere would be cleaner.
Suggested fix: Change line 129 from 'sandbox, best effort.' to 'sandbox (best effort).' to match the other two occurrences.
fullsend-ai-review
Bot
added
the
ready-for-merge
1 participant
Follow-up to #7049 (ADR 0105). Docs only.
The budget-and-deadline docs said the runner "terminates the agent's processes" at the budget. What the code does is end the exec and run the stray-process sweep, which is best effort by its own contract. OpenShell has no per-exec kill by design: an exec's processes are not expected to exit with the caller, and a runner that reuses one sandbox across execs owns their cleanup (NVIDIA/OpenShell#3159 was closed as expected usage, not as a refused feature).
docs/cli/run.md: the two sentences that claimed termination now say "sweeps the processes the agent left running, best effort", matching the rule bullet that already said so; that bullet gains "by design" and the Close issue #2574: recommendation contradicts ADR 43 and targets relocated code #3159 link. The console blocks are real output and stay as pasted.docs/reference/harness-reference.md: same wording ontimeout_minutes.docs/ADRs/0105: a short dated note after Consequences connecting the Close issue #2574: recommendation contradicts ADR 43 and targets relocated code #3159 citation to that reading. The Decision text is untouched; whether the budget kill moves off the sweep is a follow-on decision, not this PR.Refs #7042.