feat(shell): improve background job control - #3673

Greptile Summary

The PR improves persistent-shell stdin isolation and expands background-job management with waiting, incremental output, and reader-drain tracking.

• Rewrites asynchronous shell lists so commands cannot consume later persistent-shell input.
• Adds timeout-aware wait and incremental output --new controls.
• Retains completed jobs while inherited output pipes are still draining.

Confidence Score: 4/5

The PR is not yet safe to merge because a normal jobs or subsequent bg operation can delete a completed job before its newly drained output is collected.

The drain fix preserves jobs only while the reader thread is alive; once it terminates, cleanup invoked by listing or starting jobs removes the buffered output even if no caller has read it.

Files Needing Attention: gptme/tools/shell_background.py

Important Files Changed

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Background parent exits] --> B{Reader still draining?}
  B -- Yes --> C[Keep job tracked]
  C --> D[Descendant closes pipe]
  D --> E[Reader terminates]
  E --> F[Caller runs jobs or starts another job]
  F --> G[Cleanup deletes finished job]
  G --> H[Trailing buffered output becomes unavailable]
  B -- No --> F

_{Reviews (2): Last reviewed commit: "fix(shell): keep tool description under ..." | Re-trigger Greptile}

Codecov Report

❌ Patch coverage is 66.03774% with 72 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Pushed a maintainer commit onto this branch: 8f77fce (fix(shell): keep tool description under OpenAI 1024-char limit).

CI was red on the only real leftover: test_tool_descriptions_within_openai_limit — shell instructions were 1064 chars vs OpenAI's 1024-char function-description cap (tests.test_tools.py:637). That's the same failure as run 33198345325. The new wait / output --new docs were the overage, not the implementation.

I compressed the tool-format instructions (not the examples) so the new primitives still fit, including when to poll vs wait. Locally the description is 840 chars with this machine's program list; worst-case all 12 candidate programs still leaves ~150 chars of headroom. Focused tests: 75 passed (test_tool_descriptions_within_openai_limit, ampersand redirect cases, wait dispatch, background-job tests).

Rest of the PR looks like the right slice for #3667:

• Ampersand rewrite tests cover the actual bug (sleep 1 & cat) and the easy false positives (2>&1, &>, quoted &, heredoc).
• wait <id> [timeout] + output --new are the missing poll primitives. Timeout-leaves-it-running is the correct contract.
• Greptile P1s on stdin isolation and drain reporting look addressed in 5d40c4c.

Not merging from here — waiting on CI for 8f77fce. No competing PR.

@greptileai review

Approved the fork-PR GitHub Actions runs on 8f77fce. After the maintainer push they were sitting in action_required (only Greptile had actually executed). Test / Lint / Build / Docs / Benchmark / Security Advisories are in progress now.

Greptile's new P1 (cleanup discarding drained output) is the existing finished-job cleanup contract, not a drain regression. jobs/bg have always dropped completed jobs once the reader exits; this PR keeps them while draining. Collect-before-cleanup would be a separate design change and would break test_excludes_finished_jobs. Not changing that here.

Still not merging until CI on 8f77fce is actually green.

CI on 8f77fce is green. Self-merge not eligible here (external author, core shell files, Greptile summary still 4/5 after the leftover P1 was withdrawn as the existing finished-job cleanup contract). Waiting for human review or merge.

Erik merged this (01b6ad50a). Re-ran the #3667 symptoms on master; all three pass. Closed #3667.