fix(packaging): verify the kirocrew entry-point after venv install + self-heal a half-built venv

[timwukp]

Problem / Motivation

An interrupted editable install (pip install -e .) can leave a crew's venv with a working Python interpreter but no usable kirocrew console entry point. Nothing previously detected or repaired this state, so a later cloud-crew connect could fail when the remote auth step exited 127 with kirocrew binary not found.

Reported by @timwukp in #8409. This is the same failure class previously documented in #3220, which was resolved independently; this change is limited to the venv entry-point incident tracked in #8409.

Why it matters

A recoverable mid-install interruption can become an opaque "crew unreachable" outage: a launcher symlink can point at a missing target, the service can fail on restart, and the operator sees only code 127 without a diagnosis of which candidate was absent, dangling, or non-executable.

What changed (motivation → approach → change)

The install and update paths treated a successful subprocess or dependency import as sufficient even when the actual console entry point or target-venv package was unusable. An atomic .venv.new swap was considered and rejected because the update path has no independent interpreter to build from and Windows cannot rename the running executable this code already protects. The fix therefore enforces explicit in-place postconditions and repairs only a narrowly verified interrupted state:

• install.sh — after pip install -e . and before recording .install-method=pip, require the kirocrew script to be executable and require kiro_crew to import through the venv interpreter in isolated mode (python -I), so an inherited PYTHONPATH cannot mask a half-built install.
• dep_sync.sync_or_reinstall — after every full editable reinstall, require the platform-correct console script and run the package import through the existing isolated interpreter probe (-I, neutral cwd, inherited PYTHONPATH ignored). The intentional Windows locked-wrapper dependency-only branch still returns before these full-reinstall postconditions.
• Managed-venv ownership invariant — centralize the missing-package repair predicate around the exact <repo>/.venv platform interpreter. Reject symlinked or junction-like .venv and bin/Scripts directories, fail closed on filesystem-inspection errors, require a runnable interpreter, and allow only the final interpreter symlink used by normal POSIX venvs. Foreign paths and foreign origins remain refused.
• slack/gateway.py — run the potentially long repair through a directly owned async dep_sync.py --repair-missing-package subprocess only after the dashboard/API socket has bound. Timeout and cancellation kill the child process tree and bounded-reap it. A dedicated single-flight repair-task handle is cancelled and awaited during graceful shutdown, preventing pip from surviving the gateway that started it.
• instances/token_mint.py — before exit 127, emit bounded per-candidate diagnostics for absent paths, dangling symlinks, and present-but-non-executable candidates, including venv interpreter and entry-point sentinels. Existing transports redact and carry stderr to the operator.

Tests

• test/test_installer_python_floor.py pins the executable/import gates before .install-method is recorded, including the isolated -I spelling.
• test/test_dep_sync.py covers missing/non-executable entry points, isolated import failure and timeout, locked-wrapper behavior, exact managed-venv ownership, redirected .venv and scripts directories, a normal final interpreter symlink, foreign/unrunnable targets, and the narrow internal repair CLI.
• test/test_slack_gateway.py covers platform-correct script paths, explicit repair intent, post-bind scheduling, single-flight task ownership, child-tree kill/reap on timeout or cancellation, and shutdown cancellation/join through the real repair path.
• test/test_bootstrap.py and test/test_cli_server_more_coverage.py exercise the isolated post-install import contract through bootstrap and update callers.
• test/test_instances.py covers the ordered remote candidate diagnostics.

On exact published commit df2d2c0b2e21312e350e961cfbb8f6899174c5c0, the seven affected test files collected 897 tests: 893 passed, 4 skipped. Four targeted mutation proofs each failed in pytest call phase when installer isolation, redirected-directory refusal, cancellation tree-kill, or shutdown task ownership was removed.

Manual verification

An earlier live t4g.xlarge reproduction established the half-built-venv failure shape, installer failure behavior, and remote dangling-symlink diagnostics without changing crew data. The exact final candidate then passed repository-wide isort, flake8, mypy (1,305 source files), pinned Black 26.3.1 plus its baseline ratchet, subprocess-encoding, async-I/O, Agent SDK boundary, and lockdown gates, git diff --check, the single-commit push guard, and the complete affected-file matrix above. Focused GPT and Opus reviews of the exact SHA both passed after the shutdown-owned repair task regression was added.

Screenshots / video

Why no screenshot: packaging, gateway lifecycle, and CLI diagnostics only; no rendered UI surface changes.

Related Issues

Fixes #8409

Pattern harvest

Rule candidate: review-prompt
Pattern: flag install/build steps that verify a proxy signal instead of the artifact produced; require isolated probes, explicit filesystem ownership boundaries, and cancellation ownership for repair subprocesses.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (N/A — behavior is internal and covered by code comments/tests)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

[timwukp]

Pushed 4389d17 fixing two real regressions the Windows backend shards caught on the prior head (not flakes):

• fixed — test_bootstrap.py::test_self_heal_runs_fixed_pip_argv: _bootstrap._self_heal (a missing-dependency catch-up) routes through dep_sync.sync_or_reinstall from a build/degraded interpreter that never carried the kirocrew console script. The new post-install entry-point verification fired there and turned a legitimate dependency heal into a failure. Narrowed it to run only when the venv already had the script (had_script computed pre-install via a subprocess-free Path.is_file() stat) — i.e. only on a genuine repair of an existing install, which is the half-built-venv case the guard exists to catch. Added test_sync_or_reinstall_skips_entrypoint_verification_when_no_script_existed to lock the narrowing in.
• fixed — test_security_posture.py::TestGateSideLogRedactorSpelling: the new _check_console_script gate-side logger.error is baseline-redactor site fix(sandbox): bypass toolbox shim to fix nested sandbox failure on macOS 26 #7 in slack/gateway.py; the census recorded 6. Raised it to 7 — same class and process as the existing dep-repair log line already in the census (OSS gateway boot self-heal path).

Local gate green: 129 tests pass across test_dep_sync.py, test_bootstrap.py, test_installer_python_floor.py, test_instances.py::TestTokenMintGeneric, test_security_posture.py::TestGateSideLogRedactorSpelling; flake8 (max-line 100) clean.

Note: the shard-3 os.killpg/pwsh-timeout failures in test_playwright_cli_installer.py are pre-existing Windows-platform issues untouched by this diff (it changes no installer/PowerShell code); they should not recur as a function of this PR.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of d359401d048836f06259051c7ff2f163b87adee4 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound self-heal with well-argued fail-closed ownership, but the background pip has no mutual exclusion with other installers and its failure remedy points at a broken command.

Watch

• The repair pip is unsynchronized with the other pip writers on the same venv. _schedule_console_script_repair is single-flight only against itself; it runs post-bind for up to _DEP_INSTALL_TIMEOUT_SECS while the dashboard update handler (dashboard/handlers/updates.py:1382) and kirocrew update (cli_server.py:1406) can each start their own sync_or_reinstall pip against the same .venv. Two concurrent pips interleaving writes can reproduce exactly the half-built state this PR exists to repair — and a broken install is precisely when an operator will click "update." A shared lock (file lock or a gateway-level "install in flight" guard) would close it.
• Every failure branch prints "run manually: kirocrew update", but in the motivating state that remedy doesn't work. When the console script is missing the kirocrew command may not exist, and when the package is absent (installed_package_origin → None), ordinary sync_or_reinstall REFUSES without the repair flag — this PR deliberately keeps it that way (test_sync_or_reinstall_still_refuses_absent_package_without_repair_intent). So the sandbox-unavailable/timeout/failure operator is directed into a command-not-found or a refusal loop; the accurate remedy for this state is re-running the installer (or an operator-facing repair spelling).

[DESIGN-REVIEWED] d359401

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of d359401d048836f06259051c7ff2f163b87adee4 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified against the base. kiro_crew/__init__.py imports only stdlib at top level, so the new import kiro_crew -I guard does not subsume the existing aiohttp probe — which remains the one non-isolated dependency probe (grep -c "import in install.sh: line 506) in the same record gate the change hardens. The existing _check_missing_deps repairs dependency distributions only and cannot rewrite the entry point, so the new startup repair is not a second spelling. Base has 4 sync_or_reinstall callers (cli_server.py:1406, slack/gateway.py:10368, dashboard/handlers/updates.py:1382, _bootstrap.py:101); the new CLI branch makes 5, matching the updated comment. All the sandbox and kill/reap seams the gateway code uses exist in the base. One extra supporting fact: install.sh pipes pip through tail inside a backgrounded subshell, so wait $! reports tail's status, not pip's — the new artifact postcondition guards a genuinely maskable proxy signal.

First-Principles-Verdict: CONCERNS

The change's own "PYTHONPATH cannot mask a half-built install" rationale is not applied to the aiohttp probe three lines above, inside the same record gate.

What this change ships

Intent: make an interrupted editable install detectable and self-healing so cloud-crew connect stops dying with an opaque exit 127 — a FIX.

1. Installer aborts before recording .install-method=pip if the kirocrew entry point is missing — justified
2. Installer aborts if kiro_crew won't import in isolated mode — justified, but see Watch
3. Full reinstall fails loudly when pip returns 0 but script/package is unusable — justified (wait $! masks pip's status behind tail)
4. Gateway auto-repairs a missing console script at startup, backgrounded after socket bind — justified (the fix for Interrupted venv reinstall leaves a half-built venv with no self-heal; remote token mint fails with an undiagnosable exit 127 #8409)
5. Shutdown cancels the repair and kills its pip tree — justified
6. Repair refused for symlinked/junction .venv/scripts, foreign target, unrunnable interpreter — justified (agent-untrusted boundary)
7. Repair skipped with a named reason when no sandbox backend exists — justified (untrusted-bytes boundary)
8. New dep_sync --repair-missing-package CLI mode — one consumer (gateway), minimal singular form, justified
9. Remote exit-127 now prints per-candidate diagnosis — declared, justified by the opaque-127 report in Interrupted venv reinstall leaves a half-built venv with no self-heal; remote token mint fails with an undiagnosable exit 127 #8409
10. interpreter_version grows an optional timeout — undeclared plumbing, one consumer

Watch

The description says the -I import guard exists "so an inherited PYTHONPATH cannot mask a half-built install", yet install.sh:506 "$_venv/bin/python" -c "import aiohttp" — the dependency half of the same .install-method gate — still runs without -I and is maskable the same way. Counted: 1 unfixed sibling (grep -c "import in install.sh; the other hits probe sys only).

Subtractions

Merge the two probes: replace the unisolated aiohttp check at install.sh:506 and the new import kiro_crew guard with one isolated probe ("$_venv/bin/python" -I -c "import kiro_crew, aiohttp") — two subprocess probes become one and the masking hole closes as a side effect.

[FIRST-PRINCIPLES-REVIEWED] d359401

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed d359401d048836f06259051c7ff2f163b87adee4 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] d359401

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed d359401d048836f06259051c7ff2f163b87adee4 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] d359401

[timwukp]

• fixed — Repair blocks gateway readiness (span=e51cd2330ac2)

The earlier rebuttal is superseded: after rebasing onto fresh upstream and inspecting GatewayOrchestrator.run, the concern was valid. The repair had been awaited before either dashboard/API socket bind and could spend the full 300-second pip timeout there.

Published head eb09d7e16892067496770cac599a39052d54905b schedules the repair as a strongly referenced background task only after _init_dashboard() or _init_api_server() returns. The task re-raises cancellation, logs other failures, and is removed from _background_tasks on completion. The source-order regression and tracked-task lifecycle tests pass; moving scheduling back before bind is killed in pytest call phase.

[timwukp]

• fixed — unreachable executable diagnostic branches

Published head eb09d7e16892067496770cac599a39052d54905b removes the symlink -> … (executable) and present, executable branches. The diagnostic block runs only after [ -x "$b" ] has already failed, and -x follows symlinks, so those states were unreachable except for a race. Tests explicitly reject both obsolete messages while retaining absent, dangling, and present-not-executable diagnostics.

[timwukp]

• fixed — unrelated test_bootstrap.py formatting hunks

Published head eb09d7e16892067496770cac599a39052d54905b restores the three formatting-only hunks to their upstream form and keeps test/test_bootstrap.py in .github/black-baseline.txt. Only behavioral fake updates required by the isolated post-install probe remain. Pinned Black 26.3.1 and the baseline ratchet pass without broad formatting churn.

[timwukp]

• rebutted — execute the wrapper during gateway boot to detect a stale shebang

The suggestion holds as broader hardening but is disproportional to this PR's reported incident and recovery boundary. #8409 is the absent/non-executable wrapper state after an interrupted venv rebuild; the gateway repairs exactly that artifact state. Executing the wrapper during every boot would add a new child-process probe and a wider behavioral contract to the readiness path.

The full reinstall already verifies the target interpreter's package import through the isolated -I probe, and the wrapper path/executable bit is checked platform-correctly. Keeping wrapper execution out of boot avoids expanding this fix into speculative stale-shebang detection while leaving the actual interrupted-rebuild incident complete.

[timwukp]

• fixed — Import check can be satisfied outside the venv (span=0db647b504f3)

Published head df2d2c0b2e21312e350e961cfbb8f6899174c5c0 runs the installer postcondition as "$_venv/bin/python" -I -c "import kiro_crew", preventing inherited PYTHONPATH from satisfying the probe outside the managed venv. The installer regression asserts isolated mode and rejects the previous unsafe spelling; removing -I is killed by the targeted mutation proof.

[timwukp]

• fixed — Repair subprocess survives gateway shutdown (span=e51cd2330ac2)

Published head df2d2c0b2e21312e350e961cfbb8f6899174c5c0 replaces executor repair with a directly owned async dep_sync.py --repair-missing-package subprocess. Timeout and CancelledError kill its process tree and bounded-reap it; a dedicated single-flight repair-task handle is explicitly cancelled and awaited by _shutdown(). Regressions exercise the actual repair/shutdown path and prove cancellation, kill, reap, and task-handle cleanup; omitting either cancellation tree-kill or shutdown ownership is killed by targeted mutation proofs.

[timwukp]

• fixed — Lexical path check can overwrite an unrelated venv (span=2f4d81a0dda6)

Published head df2d2c0b2e21312e350e961cfbb8f6899174c5c0 centralizes missing-package repair ownership in _is_owned_project_venv_target. It rejects symlinked or junction-like .venv and bin/Scripts directories, fails closed on inspection errors, requires the exact runnable managed interpreter, and deliberately permits only the normal final POSIX interpreter symlink. Tests cover redirected directories, standard layouts, a final interpreter symlink, foreign targets, and unrunnable interpreters; weakening redirected-directory refusal is killed by the targeted mutation proof.

[timwukp]

• fixed — Startup repair executes an agent-writable venv interpreter unsandboxed (src/kiro_crew/slack/gateway.py:2541)

Published head d76a7c4051a907305e1ee1a6160f14f049bb6372 routes the repair spawn through the sandbox chokepoint. The finding holds and I am not contesting it: the spawn's own argv runs the gateway's trusted sys.executable, but it passes venv_py onward and dep_sync then EXECUTES that interpreter via installed_package_origin → _probe_interpreter (dep_sync.py:412,468), so the repair subtree ran with no filesystem isolation and an unscrubbed environment. Round 3's _is_owned_project_venv_target constrains WHICH path may be repaired; it cannot constrain the bytes at that path, which is what this finding is about.

The repair now calls sandboxed_spawn_argv_async(..., mode="standard", strip_python_env=True, _prepare=sandboxed_spawn_argv) and spawns the prepared argv via create_subprocess_limited, so the child gets the OS sandbox, the credential-scrubbed env, and the kernel/cgroup resource ceiling. mode="standard" is deliberate: it masks the credential dirs while leaving the project and its venv writable, which the repair requires because pip install -e . is the one operation that rewrites the entry point. No extra_writable_dirs is passed — the project is already writable under this mode, and a carve-out outside the sealed runtime parent is refused by _writable_carveout_spellings anyway, so passing one would only look like protection.

I first tested the opposite disposition and abandoned it. The sibling _check_missing_deps is already in BENIGN_SPAWNS, which looked like a precedent for allowlisting this one. It is not: that function spawns sys.executable -m pip install <missing> and never passes or executes venv_py. Allowlisting _check_console_script would have asserted "command, arguments and working directory are NOT agent-influenced", which is false for exactly the reason this finding gives.

SandboxUnavailableError fails CLOSED rather than falling back to a bare spawn — a host with no sandbox backend keeps the pre-existing manual path (kirocrew update) instead of gaining an unsandboxed execution of an untrusted interpreter. The skip logs the typed kind and probe detail rather than an inferred guess, since an undiagnosable failure of this exact repair is what #8409 is about. I deliberately did NOT set first_party_fixed_argv=True: it would require a second allowlist entry claiming zero user-config influence over an argv that carries KIROCREW_PROJECT_DIR, and it would fail OPEN precisely on the backend-less hosts where this exposure would land.

The repository's own tripwire agreed with you independently: test_spawn_audit.py::test_every_spawn_is_routed_or_allowlisted was red on df2d2c0b naming slack/gateway.py::_check_console_script, and Backend Tests (3.12, 4), Backend Tests (Windows) (4) and Coverage Gate were the same single assertion plus its fail-closed cascade. Routing turns all four green together.

Cancellation is preserved, which I verified rather than assumed: both sandbox wrappers and the rlimit shim exec in place under the caller's setsid session, so start_new_session=platform_compat.IS_POSIX is retained and killpg(getpgid(proc.pid)) still reaches pip and its build-backend grandchildren. The launcher temp file is unlinked in a finally, and the cancellation test now asserts it does not leak.

Verification on the published head: 457 tests pass across test_slack_gateway.py, test_spawn_audit.py, test_dep_sync.py and test_security_posture.py; all 12 spawn-audit tests green; pinned black==26.3.1, flake8, pinned isort==6.0.0 over CI's exact scope, and mypy==1.14.1 all clean. Two mutation proofs kill the fix: reverting create_subprocess_limited to a bare create_subprocess_exec fails both the new regression test and test_every_routed_spawn_applies_resource_limits, and stripping the sandbox preparation entirely fails five tests including the repo's own audit.

[timwukp]

• fixed — Repair exposes credentials to an agent-controlled interpreter; run with mode="strict" (src/kiro_crew/slack/gateway.py)

Published head d359401d048836f06259051c7ff2f163b87adee4 changes the repair spawn from mode="standard" to mode="strict", exactly as the finding asks. The finding holds: the child executes venv_py, whose bytes are the project checkout's and therefore untrusted, so hiding .aws/.ssh while it runs is the right posture, not speculative hardening.

I verified strict does not break the repair before applying it, because a silently broken self-repair would be worse than the finding. strict additionally hides (vs standard) the _CC_FILES set — .npmrc, .pypirc, .netrc, .git-credentials, .env — plus .ssh (the strict-only hide_ssh flag) and .aws/.kube/.config/gh. For this repair's pip install -e . of an already-cloned project against a package index: scrub_env is mode-independent so PIP_INDEX_URL/proxy/SSL_CERT_FILE/HOME/PATH still survive; the network stays open and <proj>/<proj>/.venv stay writable (mode changes only the credential mask, never network or project writability). .aws/.ssh/.git-credentials/.config/gh are not on a plain non-VCS install's path. The one narrow break is a private index authenticated solely via ~/.netrc — implausible for a KiroCrew self-repair, and if it ever arises the lever is an extra_visible_dirs carve-out, not a looser mode.

I considered mode="cc" as a middle ground and rejected it: cc masks the same .pypirc/.netrc yet leaves .ssh and .aws/config visible, so it is strictly worse than strict for this finding's goal. strict is the tightest tier that hides both .aws and .ssh.

Rebased onto current origin/main (cbdd4a56) in the same revision. That absorbed #9182, which repaired test_security.py::test_chained_cd_expansions_do_not_blow_up_the_gate after #9089 removed security._dir_holds_sensitive_leaf. The previous head d76a7c40 sat on the pre-#9182 base d4cb9afc, so that test (and the Windows shard-3/4 + Coverage Gate cascade) was an inherited base red, not introduced by this PR — the rebase clears it.

Verification on the published head: 331 tests pass across test_slack_gateway.py, test_spawn_audit.py, and the previously-red test_security.py chained-cd test; spawn-audit 12/12 green; pinned black==26.3.1, flake8, isort==6.0.0 over CI's scope, and mypy==1.14.1 all clean; one squashed commit, diff parity confirmed (11 files, +1137/-19, 0 behind base), install.sh mode preserved at 100755.

[bolichen97]

Tech Lead review — approved.

Self-heal safety. The repair is an in-place pip install -e .; nothing is deleted, moved, or swapped, so there is no path to user-data loss. The ownership bypass that lets the reinstall proceed with no installed package is narrow on four independent conditions: the opt-in allow_missing_package_repair flag (set only by the internal --repair-missing-package CLI), origin is None, exact normcase/abspath equality with <repo>/.venv's platform interpreter, and a runnable interpreter. _is_owned_project_venv_target additionally refuses a symlinked or junction-like .venv or bin/Scripts, and fails closed on OSError — so lexical equality cannot be turned into authority over another tree. Path.is_junction is reached through getattr, so the 3.10/3.11 shards are safe.

No masking of a real failure. This is the inverse: the change replaces proxy signals with artifact postconditions. install.sh and sync_or_reinstall both require an executable console script and an isolated (-I) import kiro_crew through the target interpreter before declaring success, so pip returning 0 on a half-built venv now returns 1. The gateway path never heals into a broken state silently — failure, timeout, and SandboxUnavailableError each print a named reason plus the manual kirocrew update fallback and log redacted detail. The Windows locked-wrapper dependency-only branch still returns before the full-reinstall postconditions, so the substitute path is not asked to rewrite a wrapper it deliberately leaves alone.

Cross-platform. Consistent with AGENTS.md's platform_compat rule: start_new_session=platform_compat.IS_POSIX, process-tree teardown routed through platform_compat.kill_process_tree_async (process-group signal on POSIX, taskkill /T on Windows, so the Windows child is still reaped without a session), and both console_script_path and project_venv_python split on sys.platform rather than assuming the POSIX bin/ layout — the correct fix for a module that exists for the Windows Scripts\kirocrew.exe case.

Lifecycle. Scheduling after the API socket binds keeps READY off the pip budget, and the dedicated _console_script_repair_task handle (cancelled and awaited in _shutdown, distinct from the GC-retention _background_tasks set) is what actually prevents pip outliving the gateway — the general set would not have.

Scope. 4 source files / ~340 added lines, all on the reported exit-127 chain; the token_mint.py diagnostics are the surface that produced the opaque failure in #8409, not adjacent cleanup. Single commit, 897 tests across the affected files with four mutation proofs.

Signals. All 60 check-runs success / 6 skipped / 0 failure on d359401d; GPT 5.6 and Opus 4.8 report no blocking findings against this exact SHA (comments updated 03:52–03:55, after the 03:40:48 head), Design and First Principles are 🟡 CONCERNS which are advisory by the lanes' own contract; readiness: passed applied 04:25:32, postdating the head. No unresolved blocking findings and no CHANGES_REQUESTED.

One accepted trade-off, not a blocker: mode="strict" hides credential dirs while pip runs, so an operator whose index auth lives in ~/.netrc rather than PIP_INDEX_URL may see the repair fail rather than succeed. That degrades to the explicit "run manually" message, which is the right direction for an untrusted-interpreter child.