fix(deployments): tolerate transient gateway 5xx during deploy status polling

[agent-zhang-beihai]

What was broken

insforge deployments deploy polls the deployment status endpoint every 5s until READY. pollDeployment()'s catch rethrew every CLIError, including HTTP-level errors surfaced by ossFetch — so a single transient gateway 502 on the status (or sync) request mid-poll aborted the command with Error: OSS request failed: 502 even though the deployment itself continued and reached READY on Vercel.

What changed

In src/commands/deployments/deploy.ts pollDeployment(), the catch now only rethrows CLIErrors that are terminal:

• deployment-failure errors thrown by the poll loop itself (no statusCode), and
• 4xx responses (statusCode < 500).

CLIErrors with statusCode >= 500 are treated as transient — same as network-level fetch errors already were — and polling continues. pollDeployment is now exported for unit testing (matching collectDeploymentFiles/createZipBuffer in the same file).

Verification

• New unit test src/commands/deployments/deploy-poll.test.ts (vitest, fake timers, mocked ossFetch):
  • a mid-poll 502 is skipped and polling proceeds to READY,
  • a 404 still fails fast,
  • a deployment reporting ERROR status still fails.
• Full suite green: npm run lint → 639 tests passed / 13 skipped, eslint clean apart from a pre-existing unrelated warning in src/lib/api/apify.test.ts.

No agent-skills update needed: no flags, defaults, or output shape changed.

Addresses user feedback 81b47ca8-b7ab-4f01-89be-5ca899b37102 (cli): deployments deploy/status intermittently fail with 'OSS request failed: 502' while deployment succee

🤖 Generated with Claude Code

---

Summary by cubic

Tolerate transient 5xx/server, 408/429, and network read errors during deployment status polling, and surface why polling timed out. Fixes intermittent “OSS request failed: 502” and makes timeouts clearer (feedback 81b47ca8).

• Bug Fixes
  • In pollDeployment(), fail fast only for deployment-thrown CLIErrors (no statusCode) and non-retryable 4xx; treat 5xx, 408/429, and network failures as transient and keep polling. Clear any transient error after a successful read.
  • When the poll window closes without READY, return lastError; shown in CLI and --json to distinguish “slow build” from “status endpoint unreachable.” Exported pollDeployment(), POLL_INTERVAL_MS, POLL_TIMEOUT_MS. Added vitest tests for transient 5xx/408/429/network tolerance, 4xx fail-fast, deployment ERROR, persistent outage reporting, and lastError clearing.

^{Written for commit 262e018. Summary will update on new commits.}

Note

Tolerate transient 5xx errors during deployment status polling in pollDeployment

Previously, any CLIError thrown during polling would terminate the poll loop immediately. Now, pollDeployment in deploy.ts only terminates early for locally-thrown deployment failures (no statusCode) or client-side 4xx errors; 5xx gateway/server errors and network-level fetch errors are treated as transient and polling continues. Tests covering these three cases are added in deploy-poll.test.ts.

Changes since #212 opened

• Modified pollDeployment function to retry transient HTTP errors and track last error [2d60b54]
• Updated create and deploy command handlers to report dynamic timeout duration and last polling error on timeout [2d60b54]
• Exported poll interval and timeout constants and defined transient status codes [2d60b54]
• Added test coverage for transient error handling in deployment polling [2d60b54]
• Added symlink from node_modules to user directory [2d60b54]
• Deleted symlinked node_modules entry [45aec86]
• Changed pollDeployment to treat 5xx HTTP status codes and network-level fetch failures as transient errors that continue polling, while treating 4xx status codes and CLIError instances without statusCode as terminal errors that abort immediately [94f3a41]
• Removed POLL_TIMEOUT_MINUTES constant and hardcoded timeout warning messages to fixed strings in deployment commands [94f3a41]
• Removed test cases for previously-supported transient error scenarios and updated remaining test assertions in pollDeployment test suite [94f3a41]
• Modified pollDeployment function to retry on transient 4xx HTTP status codes and added corresponding test coverage [262e018]

^{Macroscope summarized e878664.}

[greptile-apps]

Greptile Summary

This PR makes deployment polling resilient to transient status-read failures.

• Retries gateway/server errors, network failures, HTTP 408, and HTTP 429 while polling.
• Preserves fail-fast behavior for deployment failures and other client errors.
• Reports the latest transient error when polling times out.
• Adds focused unit coverage for retry, failure, recovery, and timeout behavior.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/commands/deployments/deploy.ts	Updates deployment polling to retry transient HTTP and network failures, retain timeout context, and expose the polling helper and constants.
src/commands/deployments/deploy-poll.test.ts	Adds unit coverage for transient retries, terminal errors, deployment failures, persistent outages, and recovery after an earlier transient error.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Wait for polling interval] --> B{Self-hosted backend?}
  B -->|Yes| C[Request deployment sync]
  B -->|No| D[Read deployment status]
  C --> D
  D --> E{Request succeeded?}
  E -->|No| F{Transient error?}
  F -->|Yes| A
  F -->|No| G[Fail immediately]
  E -->|Yes| H{Deployment status}
  H -->|READY| I[Return live deployment]
  H -->|ERROR or CANCELED| G
  H -->|Still building| J{Polling window expired?}
  J -->|No| A
  J -->|Yes| K[Return timeout result]

Loading

_{Reviews (4): Last reviewed commit: "fix(deployments): treat 429/408 as trans..." | Re-trigger Greptile}

[jwfing]

Summary

Focused, well-tested fix that makes pollDeployment() tolerate transient gateway 5xx responses on the deployment status/sync endpoints while still failing fast on real terminal errors.

Requirements context

No matching spec/plan found under docs/specs/ (the repo's spec dir holds only diagnose-command and db-migrations-command designs; there is no docs/superpowers/). Assessed against the PR description and the referenced user feedback (81b47ca8) alone.

Findings

Critical

(none)

Suggestion

• Functionality — src/commands/deployments/deploy.ts:317. The new guard rethrows any CLIError with statusCode < 500, which includes 429 Too Many Requests. A 429 from the gateway mid-poll is conventionally transient and does not indicate the deployment itself failed — the same reasoning the PR applies to 5xx. If the status endpoint is ever rate-limited during a long poll, this would abort the command even though the deployment is still progressing. Consider treating 429 (and possibly 408) as transient alongside >= 500. Low blast radius, so non-blocking.

Information

• Software engineering — src/commands/deployments/deploy-poll.test.ts. The three tests cover the fix's branches cleanly (5xx skip-to-READY, 4xx fail-fast, deployment ERROR), and the fake-timer usage is correct (each iteration awaits the interval before fetching, so advanceTimersByTimeAsync(POLL_INTERVAL_MS * 3) maps to exactly three polls). One uncovered path worth a follow-up test: a persistent 5xx that never recovers should fall through to the timeout and return isReady: false (surfacing the "did not finish within 5 minutes" message at deploy.ts:527-539) rather than throwing — this is the user-visible behavior change and currently has no direct assertion. Not required to merge.
• The pollDeployment export, the mock path (../../lib/api/oss.js matches the import in deploy.ts:8), and the vi.hoisted + vi.mock pattern are all consistent with existing repo conventions (metadata-slug.test.ts, collectDeploymentFiles/createZipBuffer exports).

Dimensions with no findings

• Security — no security-relevant changes; no new user input reaches SQL/shell/HTTP, no secrets/PII newly logged, auth is untouched, and no dependencies were added.
• Performance — no new queries, loops, or blocking work; poll interval/timeout are unchanged. The change only narrows which errors are rethrown.

Verdict

approved (informational — human approval via the separate approve flow). The fix is correct, in-scope, and matches its tests; the 429 note and the persistent-5xx test are optional improvements the author may take or leave.

[jwfing]

LGTM - approved.

[jwfing]

Review: fix(deployments): tolerate transient gateway 5xx during deploy status polling

Summary: A well-scoped, thoroughly-tested fix that stops a single transient gateway 5xx (or 408/429/network error) from aborting deployments deploy while the deployment itself keeps running server-side, and surfaces lastError so a full-window outage is distinguishable from a slow build.

Requirements context

No matching spec/plan found under docs/specs/ (existing specs cover the diagnose and db-migrations commands only; there is no docs/superpowers/). Assessed against the PR description and the referenced user feedback 81b47ca8 ("deployments deploy/status intermittently fail with 'OSS request failed: 502' while the deployment succeeds"). The change directly addresses that intent.

Findings

Critical

(none)

Suggestion

• Functionality — app-layer 500 is retried for the entire poll window (src/commands/deployments/deploy.ts:30, :334-339). Including 500 in TRANSIENT_POLL_STATUSES means a backend whose status route hard-500s (a persistent app bug, not a gateway blip) keeps the CLI spinning for the full 5 minutes rather than failing fast like 501 does. This is a reasonable, deliberately-documented trade-off and is now mitigated by reporting lastError on timeout — flagging only so the choice is conscious. No change required.

Information

• Software engineering — excellent test coverage. deploy-poll.test.ts exercises transient 502→READY, 429 tolerance, transient sync-request failure, 404/501/no-statusCode/ERROR fail-fast, persistent-5xx and persistent-network lastError reporting, and the "slow build leaves lastError null" case. This matches the repo's TDD convention and the coverage is genuinely comprehensive. The terminal-vs-transient classification (src/commands/deployments/deploy.ts:334-336) correctly relies on ossFetch attaching res.status as CLIError.statusCode (src/lib/api/oss.ts:225) and on deployment ERROR/CANCELED CLIErrors carrying no statusCode (deploy.ts:315).
• Functionality — pre-existing wording bug fixed as a bonus. create.ts previously reported "did not finish within 2 minutes" while the real timeout is 5 min (POLL_TIMEOUT_MS = 300_000); both call sites now use POLL_TIMEOUT_MINUTES (create.ts:496, deploy.ts:580). Good consolidation.
• Functionality — minor label imprecision. When syncBeforeRead is set and the sync POST (not the status GET) fails at the network layer, formatFetchError(err, 'the deployment status endpoint') (deploy.ts:344) attributes it to the status endpoint. Harmless for user messaging; noting only for accuracy.
• Security: no security-relevant changes — no new user input reaches SQL/shell/HTTP, no secrets logged (lastError carries only HTTP status / network-error text), no auth/authz changes, no new dependencies.
• Performance: no concerns — poll cadence (POLL_INTERVAL_MS) and window (POLL_TIMEOUT_MS) are unchanged; the added work per iteration is a string assignment. Retrying 500 for the whole window is bounded by the existing 5-minute timeout.

Scope note: the standalone deployments status command (status.ts) is a one-shot read, not a poller, so the "deploy/status" in the feedback refers to the status request inside the deploy loop — correctly the only site touched. The accidental node_modules symlink from an intermediate commit was cleaned up in the final HEAD (45aec86).

Verdict

approved (informational — a human still gives the explicit GitHub approval). Zero Critical findings; the implementation solves the reported problem, is well-tested, in-scope, and free of security/performance regressions. The Suggestion and Information items are non-blocking.

[jwfing]

Summary

Focused, well-tested bug fix that stops a single transient gateway 5xx (and 408/429/network read error) on the deployment status endpoint from aborting an otherwise-successful insforge deployments deploy; the diff is in scope and the logic is correct.

Requirements context

No matching spec/plan found — docs/specs/ only holds the diagnose and db-migrations designs, and there is no docs/superpowers/ directory. Assessed against the PR description, the linked feedback item (81b47ca8 — deploy/status intermittently failing with OSS request failed: 502 while the deploy actually succeeds), and surrounding code.

I verified the core assumption the fix rests on: ossFetch throws new CLIError(message, 1, err.error, res.status) (src/lib/api/oss.ts:230), so HTTP failures carry the numeric HTTP status in statusCode, and genuine network failures surface as a non-CLIError TypeError. The terminal/transient classification in pollDeployment keys off exactly these, so it behaves as intended. I also ran the new test file locally: 8/8 pass.

Findings

Critical

(none)

Suggestion

• Functionality — sibling create flow doesn't surface the new lastError (src/commands/create.ts:484-498). The insforge create command also deploys via deployProject() and has its own "still building" branch, but it neither reads result.lastError nor was it touched here (the PR description notes this follow-up was intentionally reverted to keep scope narrow). That's a reasonable call, but it means the "status endpoint unreachable vs. slow build" distinction this PR adds is only visible from deployments deploy, not from create. Worth a fast-follow for consistency. Separately, that branch's message still says "Deployment did not finish within 2 minutes" while POLL_TIMEOUT_MS is 300_000 (5 min) — pre-existing and out of this PR's scope, flagging only so it isn't lost.

Information

• Software engineering — syncBeforeRead: true transient path is untested (src/commands/deployments/deploy.ts:299-301). All new tests call pollDeployment(..., false). A 5xx thrown by the POST /sync request (legacy/self-hosted path) hits the same catch and is now treated as transient too — correct, but uncovered. A one-line case would lock it in.
• Behavior change worth noting for reviewers — persistent outage now consumes the full 5-minute window. Because 5xx/408/429/network errors no longer fail fast, a status endpoint that is down for the entire poll window will now poll until POLL_TIMEOUT_MS before returning (with lastError populated) rather than erroring immediately. This is the intended trade-off (the deployment may still be succeeding server-side) and the window is bounded, so it's not a performance concern — just a deliberate latency shift on the sad path.

Nice touches: resetting lastTransientError to null after any successful read (deploy.ts:317-319) so a slow-but-healthy build isn't mislabeled an outage, the explicit TRANSIENT_4XX_STATUSES set with a rationale comment, and the "final read succeeded → lastError null" test guarding exactly that reset. Test coverage across the transient/terminal/timeout matrix is thorough. No security-relevant changes (no new user input reaching SQL/shell/HTTP; lastError carries only backend-controlled error strings and host/error-code diagnostics, no secrets).

Verdict

approved — no Critical findings; the Suggestion/Information items are non-blocking. (Informational verdict for the bot's report; a human still gives the explicit GitHub approval.)

[jwfing]

LGTM - approved.