Skip to content

Refactor managed hook runtime and PTY spawn env ownership#1309

Draft
SSwser wants to merge 15 commits intostablyai:mainfrom
SSwser:SSwser/hook-runtime-and-pty-env
Draft

Refactor managed hook runtime and PTY spawn env ownership#1309
SSwser wants to merge 15 commits intostablyai:mainfrom
SSwser:SSwser/hook-runtime-and-pty-env

Conversation

@SSwser
Copy link
Copy Markdown
Contributor

@SSwser SSwser commented May 1, 2026

Summary

  • fix two user-visible regressions: Windows managed hook launcher execution from the published runtime path, and PTY shells missing expected CLI/PATH state in dev workflows
  • unify managed hook runtime ownership around shared launcher registration, runtime-file endpoint discovery, and build-scoped runtime publication
  • centralize PTY spawn environment resolution in main using explicit ambientEnv/envOverrides/envToDelete inputs so local and daemon providers consume the same resolved environment

Problem statement

This branch is attached to #1308.

The current behavior exposed two reproducible problems:

  1. managed hook execution on Windows could fail from the published runtime path
  2. PTY shells could come up without the expected CLI availability / PATH shaping in dev scenarios

While investigating those failures, the deeper issue turned out to be ownership drift across layers:

  • managed hook registration, launcher rendering, runtime publication, and endpoint discovery were split across too many places
  • PTY startup mixed baseline env selection, overrides, deletions, and provider-side repair logic
  • an intermediate design direction also revealed that sharing one mutable runtime root across dev and packaged builds would violate Orca's dogfood isolation model

What changed

Managed hook runtime

  • moved managed hook registration onto one shared launcher-registration contract for Claude, Codex, Cursor, and Gemini
  • moved launcher/runtime path ownership behind shared runtime-path helpers
  • updated the launcher to self-discover endpoint state from runtime files instead of depending on PTY env transport coordinates
  • kept runtime publication isolated to the active build namespace (userData-derived runtime root, e.g. orca-dev vs orca)
  • widened managed-entry cleanup so legacy managed registrations converge onto the shared launcher path

PTY environment ownership

  • replaced the ambiguous PTY env input with explicit ambientEnv, envOverrides, and envToDelete
  • resolved the spawn environment once in main before dispatching to local or daemon PTY providers
  • removed provider-side env repair / implicit re-merging so both provider paths consume the same resolved environment
  • preserved host augmentations (dev PATH injection, attribution paths, user-data scoped env) as a final main-process step

Why this is one PR

I realize this is a large change set, so I am opening it as a Draft PR specifically to get maintainer feedback on scope and direction before expecting merge.

I chose to keep the fixes together because the two visible bugs came from the same ownership problem. Splitting only the symptoms from the infrastructure changes would leave the old contracts in place and make the fix rationale harder to review.

If maintainers would prefer this split into follow-up PRs after agreeing on direction, I am happy to do that.

Testing

  • added/updated targeted tests for runtime-path resolution, shared launcher registration, launcher runtime discovery, managed-entry cleanup, and PTY env resolution flow
  • manual verification focus was Windows dev/runtime behavior, because that is where the cross-layer regression was most visible

Feedback requested

I would especially appreciate feedback on:

  • whether the shared launcher contract boundary is the right long-term shape
  • whether build-scoped runtime isolation via the active userData namespace matches project expectations
  • whether keeping the PTY env resolution in main is the preferred ownership boundary

Made with Orca 🐋

SSwser added 15 commits May 1, 2026 20:04
Move managed hook scripts to the shared appData runtime, route Windows hook launches through a bash wrapper that preserves cmd.exe flags, and keep the daemon dev PATH prepend from dropping inherited Windows PATH entries. Also record the unified runtime + PTY env target architecture in the managed hook runtime spec.
Tasks 8 and 9 (combined since they are tightly coupled):

Task 8 - Resolve PTY spawn env in main process:
- Add resolvePtySpawnEnv() in pty.ts to centralize env resolution
- Call it in pty:spawn handler before provider delegation
- Update DaemonPtyAdapter to pass ambientEnv to daemon subprocess
- Update all pty.test.ts assertions to use ambientEnv/envOverrides API
- Fix preAllocatedHandle allocation for local PTYs
- Remove buildSpawnEnv configuration from LocalPtyProvider

Task 9 - Remove PTY provider host env repair:
- LocalPtyProvider: trust caller's ambientEnv, don't merge process.env
- DaemonPtyAdapter/pty-subprocess: same - drop process.env merge
- Both still add terminal-shape defaults (TERM/COLORTERM/LANG/etc.)
- SSH spawns now get ambientEnv: { ORCA_TERMINAL_HANDLE: ... }
- Remote agents can self-identify without full host env resolution
- Update SSH spawn test to expect terminal handle injection
- Fixes 'injects ORCA_TERMINAL_HANDLE for non-local PTY providers' test
@SSwser
Copy link
Copy Markdown
Contributor Author

SSwser commented May 1, 2026

Linked issue: #1308

I opened the issue first so the two reproducible user-visible failures and the underlying ownership problem have a stable discussion thread. This Draft PR is the concrete implementation proposal for that direction.

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