Skip to content

distinguish Amp runtime readiness from installation#527

Merged
zomux merged 1 commit into
developfrom
fix/amp-runtime-readiness
Jun 29, 2026
Merged

distinguish Amp runtime readiness from installation#527
zomux merged 1 commit into
developfrom
fix/amp-runtime-readiness

Conversation

@QuanCheng-QC

Copy link
Copy Markdown
Collaborator

Amp showed installed on the Install page but Not installed (+ X error) in the Agents list, and Connect Workspace failed with no real reason. Root cause: two independent bugs.

  1. Readiness was conflated with installation. The Install page checks the
    executable (whichBinary + enhanced PATH), but the Agents-list label is a
    readiness verdict whose amp not_ready_message literally read "Not
    installed", and formatHealthLabel showed that message whenever ready was
    false — so an installed-but-signed-out amp read "Not installed". On Windows
    the sign-in probe / Test-connection spawned amp.cmd without a shell, so it
    could never confirm login and always fell back to that message.

  2. Real workspace failures were swallowed. join/heartbeat errors were only
    logged; the daemon status carried no cause, so failures surfaced as a bare
    X error (or the misleading readiness message).

Fix — split the states with a single shared vocabulary (REASON):

  • not_installed -> ONLY when the executable cannot be resolved

  • login_required -> installed but signed out / no API key

  • ready -> installed AND (signed in OR key)

  • runtime_missing / spawn_failed / workspace_join_failed / heartbeat_failed /
    session_revoked / adapter_crashed -> real runtime failures, classified +
    redacted, written to daemon state.error_reason/last_error.

  • New src/adapters/health-status.js: reason codes, readinessReason, shouldUseShellForBinary (.cmd/.bat), redactDiagnostic, and join/heartbeat/ spawn classifiers. Single source shared by installer, adapters and daemon.

  • installer.healthCheck now emits a structured reason.

  • BaseAdapter: onStatus reporting, getExitInfo, wasStopRequested, preflight(); join/heartbeat failures classified and reported. Heartbeat only escalates to a hard error after consecutive failures, so a single transient blip / expected brief reconnect is not mislabeled.

  • AmpAdapter.preflight(): missing binary -> runtime_missing, daemon skips join. Spawn failures classify as spawn_failed (never "not installed"); diagnostics (resolved path, argv, exit/stderr) are logged and redacted.

  • daemon: preflight gate (no join when the binary is genuinely missing), live onStatus -> state, and a post-run decision that maps a clean user stop / disconnect to stopped (never error) and a real terminal failure to error with its classified reason; getStatus exposes error_reason.

  • launcher: probe + Test-connection spawn through a shell for Windows .cmd/.bat with the enhanced PATH (semantically identical to the core helper; resolution and PATH reuse the loaded core's whichBinary/getEnhancedEnv). Health methods carry reason; formatHealthLabel/tui/agent-rows key off reason/installed so "Not installed" only shows for a missing executable, and the TUI surfaces the real last_error.

  • amp registry not_ready_message reworded to a login prompt.

Other agent types are unaffected (default preflight passes; onStatus is opt-in). Tests: 6 new files covering classification/redaction, the readiness split, preflight, lifecycle status (incl. no-flap on a single blip) and the daemon mapping. NOT VERIFIED: real Windows Amp + Workspace end-to-end.

@vercel

vercel Bot commented Jun 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
openagents-workspace Ready Ready Preview, Comment Jun 28, 2026 1:37am

Request Review

Amp showed `installed` on the Install page but `Not installed` (+ `X error`)
in the Agents list, and Connect Workspace failed with no real reason. Root
cause: two independent bugs.

1) Readiness was conflated with installation. The Install page checks the
   executable (whichBinary + enhanced PATH), but the Agents-list label is a
   readiness verdict whose amp `not_ready_message` literally read "Not
   installed", and formatHealthLabel showed that message whenever `ready` was
   false — so an installed-but-signed-out amp read "Not installed". On Windows
   the sign-in probe / Test-connection spawned `amp.cmd` without a shell, so it
   could never confirm login and always fell back to that message.

2) Real workspace failures were swallowed. join/heartbeat errors were only
   logged; the daemon status carried no cause, so failures surfaced as a bare
   `X error` (or the misleading readiness message).

Fix — split the states with a single shared vocabulary (REASON):
  - not_installed  -> ONLY when the executable cannot be resolved
  - login_required -> installed but signed out / no API key
  - ready          -> installed AND (signed in OR key)
  - runtime_missing / spawn_failed / workspace_join_failed / heartbeat_failed /
    session_revoked / adapter_crashed -> real runtime failures, classified +
    redacted, written to daemon state.error_reason/last_error.

- New src/adapters/health-status.js: reason codes, readinessReason,
  shouldUseShellForBinary (.cmd/.bat), redactDiagnostic, and join/heartbeat/
  spawn classifiers. Single source shared by installer, adapters and daemon.
- installer.healthCheck now emits a structured `reason`.
- BaseAdapter: onStatus reporting, getExitInfo, wasStopRequested, preflight();
  join/heartbeat failures classified and reported. Heartbeat only escalates to a
  hard error after consecutive failures, so a single transient blip / expected
  brief reconnect is not mislabeled.
- AmpAdapter.preflight(): missing binary -> runtime_missing, daemon skips join.
  Spawn failures classify as spawn_failed (never "not installed"); diagnostics
  (resolved path, argv, exit/stderr) are logged and redacted.
- daemon: preflight gate (no join when the binary is genuinely missing), live
  onStatus -> state, and a post-run decision that maps a clean user stop /
  disconnect to `stopped` (never error) and a real terminal failure to `error`
  with its classified reason; getStatus exposes error_reason.
- launcher: probe + Test-connection spawn through a shell for Windows .cmd/.bat
  with the enhanced PATH (semantically identical to the core helper; resolution
  and PATH reuse the loaded core's whichBinary/getEnhancedEnv). Health methods
  carry `reason`; formatHealthLabel/tui/agent-rows key off reason/installed so
  "Not installed" only shows for a missing executable, and the TUI surfaces the
  real last_error.
- amp registry not_ready_message reworded to a login prompt.

Other agent types are unaffected (default preflight passes; onStatus is opt-in).
Tests: 6 new files covering classification/redaction, the readiness split,
preflight, lifecycle status (incl. no-flap on a single blip) and the daemon
mapping. NOT VERIFIED: real Windows Amp + Workspace end-to-end.

@zomux zomux left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well-architected: a shared REASON vocabulary so Install/Agents/daemon/TUI agree on why an agent is unusable. Heartbeat no-flap (threshold 2), status dedupe (guards against the #492 runaway loop), clean-stop-wins, secret redaction at every sink, en+zh i18n. Generalizes (preflight/onStatus opt-in) so non-Amp agents unaffected. Nits: launcher duplicates the .cmd shell rule instead of importing the shared helper; error_reason unused in GUI; VERSION_INCOMPATIBLE half-modeled. LGTM.

@zomux zomux merged commit 9aeda3f into develop Jun 29, 2026
5 of 14 checks passed
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.

2 participants