Skip to content

feat: improve local Bash execution contracts - #339

Open
1anZhang wants to merge 3 commits into
mainfrom
feat/bash-execution-contract
Open

1anZhang wants to merge 3 commits into
mainfrom
feat/bash-execution-contract

Conversation

@1anZhang

@1anZhang 1anZhang commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Change

Local Bash now requires exit code zero for success and preserves signals, cancellation reasons, deadlines, and partial output. Managed foreground commands can yield after 60 seconds while retaining the original 600-second command deadline. Foreground-only turns retain the 120-second default and 300-second cap.

  • Derive the Bash schema and shell guidance from admitted native tools, including background parameters only when native task_output is available.
  • Preserve original output head and tail within the 24 KiB first-response budget, keep one complete managed log, and report persistence and stop failures separately from command outcomes.
  • Carry optional command descriptions through task receipts and TUI summaries. Bound background receipt purpose text to 1 KiB using UTF-8-safe truncation while retaining task IDs, timeout information, and continuation instructions. Preserve standalone recoverable-delete routing and scoped Pi imports.
  • Reuse existing regression suites for child command lifecycles, capability gating, prompt guidance, and process outcomes. Update capability documentation and the vendor patch ledger.

Validation

  • Latest head 9cb9d8c: the existing turn-executor Vitest suite passed 141 tests. Receipt coverage exercises explicit background and automatic promotion with omitted, long UTF-8, and short descriptions, while checking original command preservation and complete control instructions.
  • pnpm typecheck, pnpm check:source (4,254 paths), and git diff --check: passed.
  • A focused probe using the real LocalBashTool and a controlled background adapter confirmed that 100 KB commands/descriptions now produce receipts of approximately 1.4 KB.
  • Full and performance GitHub CI are required on the latest head before merge; perf:full is selected.
  • Not run locally on the latest head: the full verification profile, Linux/Windows execution, live models/services, full interactive TUI acceptance, or the entire 600-second timeout. Vendored upstream suites remain outside this distribution's verification.

Publication and contribution checks

  • Shared MiniMax provenance is recorded in third_party/pi-mono/MINIMAX_CHANGES.md; existing licenses and notices are preserved.
  • No credentials, account data, real user content, internal source history, or private review material is included.
  • The six added implementation modules were reviewed before regenerating release/public-source.json; regression coverage uses existing test files and suite registrations.
  • Capability documentation and vendor records are updated. Offline verification is distinguished from live-service acceptance.

Maintainer handoff

Selective port of the applicable shared local Bash changes. The extraction baseline remains pinned. No dependencies or licenses change.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@1anZhang 1anZhang added enhancement New feature or request tui Interactive terminal UI (TUI) cli Standalone mcode: TUI, headless, ACP and source builds/tooling perf:full Run the full performance suite; required for performance-sensitive changes labels Sep 23, 2026
@1anZhang

Copy link
Copy Markdown
Collaborator Author

The performance run for 0dab5f8 failed with INCONCLUSIVE due to startup sample variability.

After five paired runs and trimming one extreme sample per revision, the candidate startup-duration spread was 38.4% and the baseline CPU-time spread was 33.0%, exceeding the 30% stability limit.

Both upstream-100 and history-300 passed all duration, CPU, and RSS checks. Their duration medians increased by 17.6% and 5.8%, respectively, within the configured regression budgets.

The full suite treats INCONCLUSIVE as a failed check (exit code 2). This run establishes neither a confirmed regression nor a performance pass. A full-suite rerun must produce PASS for the latest head and intended base before merge. Linux, macOS, and Windows verification checks have passed.

? ` Command limit: ${timing.commandTimeoutSeconds}s total; yielding does not reset it.`
: '';
const purpose =
typeof details.description === 'string' ? `Purpose: ${details.description}\n` : '';

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Bound the purpose text in background receipts

When the optional description is omitted, resolveBashDescription() falls back to the entire command, which is now interpolated into Purpose without any output limit. Both explicit background starts and automatic promotion therefore echo large inline scripts back into the model context and can exceed the documented 24 KiB first-response budget. At this head, invoking the real LocalBashTool with a controlled admission adapter and an approximately 40 KB inline script produced a 40,355-byte receipt; a 100,000-character command produced receipts over 100 KB on both paths. Please bound the displayed purpose independently, including explicitly supplied descriptions, while preserving the task ID, timeout, and continuation instructions, and add coverage for both receipt paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli Standalone mcode: TUI, headless, ACP and source builds/tooling enhancement New feature or request perf:full Run the full performance suite; required for performance-sensitive changes tui Interactive terminal UI (TUI)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants