Skip to content

fix startup command whitespace preservation - #57

Open
ooiuuii wants to merge 1 commit into
tt-a1i:mainfrom
ooiuuii:fix/startup-command-whitespace
Open

ooiuuii wants to merge 1 commit into
tt-a1i:mainfrom
ooiuuii:fix/startup-command-whitespace

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Oct 7, 2026

Copy link
Copy Markdown

Why

Custom startup commands lose shell-significant whitespace before launch. A harmless printf ending in backslash-plus-space prints a trailing space when executed directly, but Hive's real PTY prints a backslash because the resolver/parser trim the payload.

Forward the original string through the worker resolver, parser and ordinary orchestrator path. Keep trimming only for empty-input checks and CLI/preset identification; shell flags, argument augmentation, permissions and recovery policies are unchanged.

Testing

  • Real loopback HTTP, temporary SQLite and native PTY cover both worker and orchestrator launches, exact output bytes and stored argv.
  • Baseline: four failures, including both native launch paths; fixed: 27 passed.
  • Product-only revert reproduced the same failures; restore passed.
  • Focused launch/parser/route suites: 46 passed.
  • After two test-only review refinements: 18 affected tests passed, final pnpm check and git diff --check passed. Product files were unchanged by those refinements; the earlier pnpm build passed.
  • Blank worker commands retain HTTP 201/stopped behavior, no launch/run, and agent_start.ok=false.
  • A Windows parser unit checks exact padded payload forwarding. Native Windows execution is unverified; POSIX integration cases skip there.
  • Full suite was not run under the repository's risk-based guidance.
  • Initial default-pnpm launcher failure occurred before tests and is not target RED; validation used the already-installed pinned toolchain. Existing orchestrator tests logged EROFS on temporary Claude trust-file writes. No permission change or retry was made; those warnings are not attributed to this patch.

The old parser assertion now expects the original padded command instead of normalized text, matching the corrected execution contract.

Self-Review

Four independent scoped reviews: architecture A, correctness A-, test quality B+ improved to A-, spec alignment A. No blocking product finding.

Accepted test findings were fixed: cleanup now attempts every server close and restores environment/fixtures in finally while surfacing failures; a Windows padded-payload unit was added. Reviewer C rechecked only those changes. Other source reviews remain applicable because product hashes did not change. Aggregate A-. Reviewers did not independently rerun runtime tests; command receipts and validation evidence are retained.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant