diagnose incident: cloud-side answer to "why is my project 504ing / down"

[tonychang04]

What

insforge diagnose incident — asks the platform (not the instance) why the project is down or returning gateway timeouts, so it works even while the box is wedged or dead — exactly when diagnose logs stops answering.

Backend counterpart: InsForge/insforge-cloud-backend#814 (GET /projects/v1/:id/diagnose/incident). Output = verdict (paused / platform operation / OOM likely / down unknown / no incident), plain-language explanation, the facts behind it, and what to do next. --json passes the raw report through.

Verdict: Out of memory (likely)

The instance stopped reporting at 2026-08-10T14:32:00Z with memory at 96.4% right before ...

  - Instance metrics: not reporting (last seen 8/10/2026, 2:32 PM)
  - Database connection: unreachable
  - Down since: 8/10/2026, 2:32 PM
  - Memory right before: 96.4%
  - Instance type: nano

What to do: Upgrade to a larger instance: `npx -y @insforge/cli projects upgrade-instance <type> --yes` ...

Notes

• Platform login required (--api-key link mode rejected, same as diagnose metrics)
• Merge after the backend PR deploys; until then the endpoint 404s
• npm run build green; npx tsc --noEmit has one pre-existing error in src/lib/prompts.ts that reproduces identically on pristine main (clack types drift), untouched by this PR

🤖 Generated with Claude Code

https://claude.ai/code/session_016kbhm3x6R6B4vy5Z2H8mKb

---

Summary by cubic

Adds a cloud-side diagnose incident command to explain why a project is down or 504ing, even when the instance is unreachable. Human-readable output shows full evidence; --json returns the exact server payload.

• New Features

  • New diagnose incident subcommand calling GET /projects/v1/:id/diagnose/incident, registered under diagnose with analytics.
  • Human-readable report shows verdict, explanation, evidence (project status, in-flight operations, recent platform operations), and recommendation; verdict labels include metrics_stopped; --json is a raw passthrough.
  • Defensive payload normalization with a clear CLIError only when the payload isn’t a report; 7 unit tests; README entry; removed legacy telemetry (kept trackDiagnose).

• Migration

  • Requires Platform login; not available with --api-key link mode.
  • Merge after the backend endpoint is deployed to avoid 404s.

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

Note

Add diagnose incident subcommand to explain project 504s and downtime

• Adds a new insforge diagnose incident subcommand in src/commands/diagnose/incident.ts that fetches /projects/v1/{project_id}/diagnose/incident from the platform API.
• Outputs either raw JSON (--json) or a human-readable report covering verdict, metrics reporting status, DB connectivity, memory usage, Postgres start time, scrape gaps, and a recommendation.
• Rejects runs when the project is not linked or is linked via API key (FAKE_PROJECT_ID), emitting a CLIError in both cases.
• Tracks usage analytics under cli.diagnose.incident on success and failure.

Changes since #228 opened

• Implemented defensive normalization of incident report API responses [482caf5]
• Removed telemetry reporting from diagnose incident command [482caf5]
• Clarified authentication requirements for diagnose incident command in documentation [482caf5]
• Added test coverage for incident report normalization [482caf5]
• Modified registerDiagnoseIncidentCommand to output raw server JSON payload when invoked with --json flag, while maintaining normalized and formatted output for human-readable mode [8bbef47]
• Added 'metrics_stopped' verdict label mapping to VERDICT_LABELS constant [be0726a]

^{Macroscope summarized 007bbb9.}

Summary by CodeRabbit

• New Features
  • Added a diagnose incident command for investigating service incidents.
  • Displays connectivity, downtime, memory, database, scrape-gap, and instance details.
  • Supports both human-readable output and JSON format.
  • Provides clear success and error reporting during diagnostics.

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[coderabbitai]

Walkthrough

Adds the diagnose incident command. The command validates authentication and project requirements, retrieves a cloud incident report, supports JSON and text output, reports usage, handles errors, and shuts down analytics.

Changes

Incident diagnostics

Layer / File(s)	Summary
Incident report retrieval and output src/commands/diagnose/incident.ts	Adds the incident report model and command flow for authentication, project validation, platform API retrieval, JSON or human-readable output, analytics, and error handling.
Diagnose command registration src/commands/diagnose/index.ts	Imports and registers the incident diagnostics subcommand with the existing diagnose commands.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: carmendou

Poem

A rabbit checks the cloud report,
With carrots packed for every port.
JSON blooms and verdicts shine,
Errors hop into a neat line.
Analytics waves goodbye—
“Diagnose incident!” echoes high.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the new diagnose incident command and its purpose of explaining project outages and gateway timeouts.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/diagnose-incident

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[jwfing]

Summary
This PR is close, but the non-JSON renderer currently omits some of the backend evidence that the new diagnose incident command is supposed to surface.

Requirements Context
I based intent on the PR description and the existing diagnose documentation in README.md, because I did not find any repo-local spec or README section for diagnose incident itself. The PR description says this command should explain why a project is down/504ing, including the verdict, plain-language explanation, the facts behind it, and next steps, while --json passes through the raw report.

Findings
Critical

• src/commands/diagnose/incident.ts:15-33,82-109 The backend contract includes incident-specific evidence fields such as project_status, operation_status, and recent_platform_operations, but the human-readable output never prints any of them. It only renders reachability, timestamps, memory, scrape gaps, and instance type. That means verdicts like paused_or_suspended or platform_operation_in_progress lose the very facts that explain why the project is down, so the command does not fully meet the PR’s stated requirement to show the “facts behind” the incident.

Suggestion

• src/commands/diagnose/incident.ts:49-124 There is no automated coverage for the new command path. A focused test around --json, the FAKE_PROJECT_ID rejection, and at least one non-JSON incident payload (especially a platform-operation verdict) would make the output contract much safer to evolve.
• README.md:1113-1143, src/commands/diagnose/index.ts:379-383 The public docs still list only advisor, db, logs, and metrics under diagnose. Adding diagnose incident to the README would keep the CLI’s documented surface aligned with the shipped commands.

Information

• (none)

Verdict
request_changes — one correctness issue blocks approval; I did not identify any new security-relevant or performance-relevant issues in this diff.

[greptile-apps]

Greptile Summary

The PR adds a platform-backed diagnose incident command for diagnosing downtime when the project instance is unreachable.

• Registers the new incident subcommand and its authenticated Platform API request.
• Adds raw JSON passthrough and normalized human-readable report rendering.
• Adds unit coverage and README usage documentation.

Confidence Score: 4/5

The PR is not yet safe to merge because active project reports still omit project-status evidence from human-readable output.

The formatter continues to suppress project_status whenever its value is active, leaving the previously reported output-contract failure outstanding.

Files Needing Attention: src/commands/diagnose/incident.ts

Important Files Changed

Filename	Overview
src/commands/diagnose/incident.ts	Implements fetching, normalization, and rendering for incident reports; the previously reported omission of active project status remains.
src/commands/diagnose/incident.test.ts	Covers report normalization and major rendering paths but does not establish that active project status is rendered.
src/commands/diagnose/index.ts	Registers the new incident subcommand under the existing diagnose command group.
README.md	Documents incident diagnosis, authentication requirements, and JSON usage.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI
    participant Platform
    User->>CLI: diagnose incident
    CLI->>CLI: Require login and linked Platform project
    CLI->>Platform: GET /projects/v1/:id/diagnose/incident
    Platform-->>CLI: Incident report
    alt --json
        CLI-->>User: Raw server payload
    else Human-readable
        CLI->>CLI: Normalize and format report
        CLI-->>User: Verdict, evidence, and recommendation
    end

Loading

_{Reviews (5): Last reviewed commit: "label the metrics_stopped verdict added ..." | Re-trigger Greptile}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/commands/diagnose/incident.ts (1)

93-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Surface project_status and recent_platform_operations in the text output.

The report includes project_status, operation_status, and recent_platform_operations, but the text branch never prints them. For the paused_or_suspended and platform_operation_in_progress verdicts, these fields carry the decisive evidence. Users who do not pass --json lose it.

♻️ Proposed additions to the facts list

+          facts.push(
+            `Project status: ${report.project_status}${report.operation_status ? ` (operation: ${report.operation_status})` : ''}`,
+          );
           if (report.down_since) {
             facts.push(`Down since: ${formatWhen(report.down_since)}`);
           }
@@
           if (report.instance_type) {
             facts.push(`Instance type: ${report.instance_type}`);
           }
+          for (const op of report.recent_platform_operations ?? []) {
+            facts.push(`Recent platform operation: ${op.action} at ${formatWhen(op.at)}`);
+          }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/commands/diagnose/incident.ts` around lines 93 - 109, Update the text
facts-building branch around the existing report fields to include
project_status and recent_platform_operations, and include operation_status
where relevant. Ensure paused_or_suspended and platform_operation_in_progress
outputs surface this evidence, while preserving the existing formatting and
conditional style used by the facts list.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/diagnose/incident.ts`:
- Line 73: After parsing the response in the incident command, validate that the
report contains a non-null object reachable before any non-JSON rendering path
reads report.reachable.metrics_reporting or report.reachable.database_connect.
Route missing or invalid reachable data through handleError with a clear API
error, while preserving normal rendering for valid IncidentReport responses.

---

Nitpick comments:
In `@src/commands/diagnose/incident.ts`:
- Around line 93-109: Update the text facts-building branch around the existing
report fields to include project_status and recent_platform_operations, and
include operation_status where relevant. Ensure paused_or_suspended and
platform_operation_in_progress outputs surface this evidence, while preserving
the existing formatting and conditional style used by the facts list.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5d8fd0f6-3baa-4052-bdc3-b242f480a47d

📥 Commits

Reviewing files that changed from the base of the PR and between bf27db7 and 007bbb9.

📒 Files selected for processing (2)

• src/commands/diagnose/incident.ts
• src/commands/diagnose/index.ts

[jwfing]

Review: diagnose incident

Summary: A clean, tightly-scoped addition of a cloud-side diagnose incident subcommand that faithfully mirrors the existing diagnose metrics pattern; no blocking issues found.

Requirements context

No matching spec/plan found — docs/superpowers/ is empty and the design docs under docs/specs/ (2026-03-27-diagnose-command-design.md, 2026-03-27-diagnose-implementation-plan.md) predate and do not mention an incident command. Assessed against the PR description and the intent stated there (platform-side "why is my project down/504ing" answer that works while the instance is unreachable), plus the backend counterpart InsForge/insforge-cloud-backend#814.

Findings

Critical

(none)

Suggestion

• Software engineering — no test coverage for the new command (src/commands/diagnose/incident.ts:1-125). There are no tests for the command. This is consistent with the existing convention — none of the sibling diagnose commands (metrics/logs/advisor/db) have command-level tests; the whole test suite lives under src/lib/. So this is not a blocker. That said, the command has non-trivial pure logic worth pinning: the fact-list assembly (incident.ts:76-104), the verdict-label fallback VERDICT_LABELS[report.verdict] ?? report.verdict (incident.ts:85), and the memory_before_down_pct vs memory_latest_pct branch (incident.ts:88-92). Extracting these into a small pure formatReport(report) helper and adding a focused vitest would guard the human-readable output against regressions without needing to mock the network.

• Functionality — backend fields returned but never surfaced in text output (src/commands/diagnose/incident.ts:16-33 vs 73-104). project_status, operation_status, and recent_platform_operations are declared on IncidentReport and returned by the platform, but only ever emitted via --json. In particular, when the verdict is platform_operation_in_progress, the human-readable output shows no operation facts at all — the user relies entirely on the backend-supplied explanation. Consider rendering the most recent entry from recent_platform_operations as a fact (e.g. Recent platform op: <action> at <when>) so the non-JSON path is self-explanatory. Low blast radius since explanation likely covers it.

Information

• Robustness of formatWhen (src/commands/diagnose/incident.ts:36-39). Only the null case is guarded; a malformed/non-ISO timestamp would render Invalid Date rather than unknown. The backend controls the format, so risk is low.
• !== null treats an absent field as present (incident.ts:88). memory_before_down_pct !== null renders undefined% if the backend ever omits the field instead of sending explicit null. The interface types it as required number | null, so this is contract-safe today — noted only in case the backend contract drifts. (scrape_gaps_24h > 0 at incident.ts:97 is safe against undefined by contrast.)
• Unguarded report.reachable dereference (incident.ts:76-83). A malformed response missing reachable would throw a TypeError rather than a clean CLIError. This matches how the siblings dereference response bodies (e.g. metrics.ts uses data.metrics directly), so it's consistent with the codebase — just flagging the shared assumption.
• Graceful 404 handling confirmed. The PR notes the endpoint 404s until the backend deploys; platformFetch (src/lib/api/platform.ts:130-135) throws a CLIError on any non-2xx, which handleError renders cleanly — so an early merge degrades to a readable error rather than a crash. Good.

Dimension notes

• Security — no concerns. Read-only GET; the path segment is the caller's own linked project_id from local config, not free-form user input; requireAuth + the FAKE_PROJECT_ID guard correctly reject --api-key link mode (matching diagnose metrics); no secrets logged (the debug path in platformFetch redacts the bearer); no new dependencies.
• Performance — no concerns. A single platform GET, no loops over the network, no N+1, trivial O(n) fact assembly.
• Conventions — clean. Imports, requireAuth(apiUrl), the FAKE_PROJECT_ID guard, trackDiagnose('incident', config), reportCliUsage('cli.diagnose.incident', …), and the try/catch/finally + handleError shape all match metrics.ts exactly. registerDiagnoseIncidentCommand is wired into registerDiagnoseCommands (index.ts:383). Correctly kept out of the aggregate diagnose report, since that path assumes instance reachability while incident is the "box is dead" fallback.

Verdict

approved (informational — no Critical findings; the human still gives the explicit GitHub approval via the approve flow). The two Suggestions are non-blocking; the strongest is adding a small unit test around the report-formatting logic.

[cubic-dev-ai]

All reported issues were addressed across 2 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[jwfing]

Review — diagnose incident

Summary: A clean, convention-following addition of a cloud-side diagnose incident subcommand that faithfully mirrors the existing diagnose metrics command; the pure formatter is well tested and there are no blocking issues, only minor robustness/test hardening opportunities.

Requirements context

No spec/plan under docs/specs/ matches this subcommand — docs/specs/2026-03-27-diagnose-command-design.md is the diagnose design of record but predates incident (it enumerates only metrics, advisor, db, logs, and the comprehensive report). There is no docs/superpowers/. I assessed against the PR description and the backend counterpart InsForge/insforge-cloud-backend#814. The implementation is consistent with the design's stated conventions (dual --json, platformFetch bearer auth, requireAuth + project-config precondition, reportCliUsage('cli.diagnose.*', ...)).

Findings

Critical

(none)

Suggestion

• Functionality / robustness — src/commands/diagnose/incident.ts:127-131. The malformed-response guard typeof report.reachable !== 'object' does not catch report.reachable === null, because typeof null === 'object'. A payload with reachable: null slips past the guard and then throws an unhelpful TypeError: Cannot read properties of null (reading 'metrics_reporting') inside formatIncidentReport (incident.ts:53), instead of the intended CLIError with the friendly "your backend may predate this command" message. Recommend tightening to !report.reachable || typeof report.reachable !== 'object'.
• Software engineering / test coverage — src/commands/diagnose/incident.test.ts. The tests thoroughly cover the pure formatIncidentReport output contract (all verdicts, forward-compat unknown verdict, project status / in-flight op / recent ops) — nice. But the .action() path is untested: the FAKE_PROJECT_ID rejection, the ProjectNotLinkedError precondition, and the malformed-response guard. A direct test of the guard would have surfaced the reachable: null hole above. Non-blocking, but a guard-path test is cheap insurance for the defensive code that exists precisely for backend/version drift.

Information

• Functionality — guard message vs. real 404 path (incident.ts:129). The guard's message ("Your backend may predate this command") only fires on a 200 response with the wrong shape. In the actual "backend not yet deployed" scenario the route 404s, and platformFetch (src/lib/api/platform.ts:131-135) throws Request failed: 404 first — so the tailored hint rarely reaches the user who most needs it. Consider mapping a 404 on this endpoint to the same "predates this command" guidance.
• Functionality — timezone rendering (incident.ts:46-49). formatWhen uses new Date(iso).toLocaleString(), which renders in the CLI host's local timezone with no TZ label. For an incident/SRE tool where operators may be comparing against UTC dashboards, an explicit timezone (or ISO/UTC) would reduce ambiguity. Tests are locale-agnostic, so this is purely a UX nicety.
• Cross-PR awareness — verdict evidence depends on backend #814. The CLI faithfully renders whatever recent_platform_operations the backend supplies. Verdict accuracy (e.g. distinguishing a genuine OOM from a platform-driven upgrade_instance/restore restart) therefore hinges on the backend sourcing those operations correctly. Worth confirming in #814 that resize/restore operations surface in recent_platform_operations, so an operation-induced restart isn't rendered as an unexplained oom_likely. Backend scope — flagged only for awareness; nothing to change here.

Notes (positive)

• Correctly reuses the shared shutdownAnalytics() (which nulls the client first, so the catch+finally double-call is intentional and safe) — matches metrics.ts exactly.
• No security concerns: project_id is a config-sourced UUID interpolated into the path (not free user input), no new dependencies, no secrets logged (bearer redaction lives in platformFetch).
• No performance concerns: a single fetch and O(n) line rendering; no loops/allocations of note.
• README entry and diagnose/index.ts registration are correct and in the right place.

Verdict

approved (informational — no Critical findings; the two Suggestions and the Information notes are non-blocking). The reachable: null guard hole is the only one I'd encourage fixing before merge, but it's low-blast-radius against your own backend. Human GitHub approval remains a separate manual step.

[jwfing]

Summary
This PR adds a useful cloud-side diagnose incident command, but the current payload validation is too shallow to support the malformed-response/backward-compatibility behavior the PR claims.

Requirements context
I based intent on the PR description first, plus the new README entry in README.md, because I did not find any separate issue/spec for this command in the repo. The PR description explicitly says this command should explain outages from the platform side, support --json passthrough, reject --api-key link mode, and include a response-shape guard with a clear CLIError on malformed payloads. I also checked DEVELOPMENT.md for command and telemetry conventions.

Findings

Critical

• src/commands/diagnose/incident.ts:129 The new "shape guard" only verifies that report is an object and report.reachable is an object. After that, formatIncidentReport() blindly iterates report.recent_platform_operations and reads other fields as if they are correctly typed. A partially deployed or malformed backend payload such as { reachable: {}, recent_platform_operations: null } will still throw a raw TypeError, and missing string fields will render as undefined, which contradicts the PR's stated requirement to surface malformed payloads as a clear CLIError. Because this command is specifically meant to keep helping when the backend/instance state is unusual, that validation needs to cover every field the renderer consumes, not just reachable.

Suggestion

• src/commands/diagnose/incident.ts:143, DEVELOPMENT.md:58 The new command still adds reportCliUsage(...), but the repo guide says not to extend that legacy telemetry path for new commands and to use PostHog only going forward.
• README.md:1146, src/commands/diagnose/incident.ts:117 The README documents the new command but omits the platform-login requirement / --api-key rejection that the PR description calls out and the implementation enforces. Users reading the CLI docs alone will only discover that limitation at runtime.

Information

• src/commands/diagnose/incident.test.ts:30 The added tests cover formatter happy paths and unknown verdict fallback, but they do not cover malformed backend payloads or the command action path, which is why the validation bug above is currently untested.
• package.json:10 I could not execute the new Vitest file in this checkout because npm test -- --run src/commands/diagnose/incident.test.ts failed with sh: vitest: not found, so this review is based on static inspection.
• No new security-relevant issues found in the changed code.
• No notable performance concerns found; this is a single platform request plus lightweight formatting.

Verdict
request_changes because the malformed-response handling does not meet the PR's stated behavior and can still fail with an unstructured runtime error.

[cubic-dev-ai]

1 issue found across 3 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/commands/diagnose/incident.ts">

<violation number="1" location="src/commands/diagnose/incident.ts:61">
P3: The `project_status !== 'active'` check silently omits the project status line from the report whenever the project is active, even though the function's own doc comment promises every evidence field the backend sends will be shown alongside the verdict. This means healthy/active-project incident reports never display the project status fact.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: The project_status !== 'active' check silently omits the project status line from the report whenever the project is active, even though the function's own doc comment promises every evidence field the backend sends will be shown alongside the verdict. This means healthy/active-project incident reports never display the project status fact.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/diagnose/incident.ts, line 61:

<comment>The `project_status !== 'active'` check silently omits the project status line from the report whenever the project is active, even though the function's own doc comment promises every evidence field the backend sends will be shown alongside the verdict. This means healthy/active-project incident reports never display the project status fact.</comment>

<file context>
@@ -46,6 +46,62 @@ function formatWhen(iso: string | null): string {
+  lines.push(report.explanation);
+  lines.push('');
+  const facts: string[] = [];
+  if (report.project_status !== 'active') {
+    facts.push(`Project status: ${report.project_status}`);
+  }
</file context>

[jwfing]

Summary
This PR adds a useful diagnose incident command, but the current --json behavior does not match the stated contract and should be fixed before merge.

Requirements Context
I based this review on the PR description and the new README entry in README.md, because I did not find a separate issue/spec in the repo documenting this command. The stated intent is: add insforge diagnose incident, have it work from the platform side when the instance is down, render a human-readable report, and have --json pass the raw backend report through unchanged.

Findings
Critical

• src/commands/diagnose/incident.ts: --json is advertised in the PR description as a raw passthrough, but the command always runs the response through normalizeIncidentReport() before calling outputJson() (src/commands/diagnose/incident.ts). That normalization rewrites omitted fields to defaults, drops malformed recent_platform_operations entries, and discards any backend fields not listed in IncidentReport. For machine consumers, this is a contract break: --json no longer returns the exact server payload, and future backend additions will be silently lost.

Suggestion

• src/commands/diagnose/incident.test.ts: test coverage only exercises the pure formatter/normalizer helpers, not the command action itself (src/commands/diagnose/incident.ts). That leaves the endpoint path, auth/link-mode gating, and especially the --json passthrough contract untested. A command-level test would have caught the blocker above.

Information

• src/commands/diagnose/incident.ts: no security-relevant issues stood out. The command uses the existing requireAuth/platformFetch path and does not introduce new secret handling or unsafe input flows.
• src/commands/diagnose/incident.ts: no material performance concerns stood out. The implementation is a single platform request plus bounded local formatting.

Verdict
request_changes because the --json output does not satisfy the PR’s documented contract.

[jwfing]

Review: diagnose incident — cloud-side "why is my project down/504ing?"

Summary: A well-scoped, defensively-written addition that closely mirrors the existing diagnose metrics command; no blocking issues found.

Requirements context

No matching spec/plan under docs/specs/ — the two diagnose docs (2026-03-27-diagnose-command-design.md, 2026-03-27-diagnose-implementation-plan.md) predate this subcommand and contain no mention of incident/verdict/504/OOM. Assessed against the PR description and the sibling command (metrics.ts) as the source of intent. Backend counterpart is InsForge/insforge-cloud-backend#814 (GET /projects/v1/:id/diagnose/incident), which is not verifiable from this repo.

Findings

Critical

(none)

Suggestion

• Functionality / cross-repo contract — src/commands/diagnose/incident.ts:36-42. VERDICT_LABELS hard-codes the key paused_or_suspended, but the tested/PR-documented verdict family uses platform_operation_in_progress, oom_likely, down_unknown, no_incident_detected. Please confirm paused_or_suspended matches the exact string backend #814 emits for the paused case. This isn't a crash risk — an unrecognized verdict degrades gracefully to the raw string (there's even a forward-compat test for that at incident.test.ts:56-59) — but a silent label mismatch would ship a rougher message than intended. Worth a one-line check against the backend enum before merge.

Information

• Functionality — pre-deploy 404 UX, incident.ts:195-199 + src/lib/api/platform.ts:131-135. platformFetch throws a generic CLIError (err.error ?? "Request failed: 404") on any non-2xx before the response reaches normalizeIncidentReport, since no passThroughStatuses is passed. The friendly "Unexpected response from the platform … Your backend may predate this command" message therefore only fires on a 200-with-wrong-body — not on the realistic "backend not deployed yet → 404" case the PR notes calls out. If you want that guidance to actually surface pre-deploy, pass passThroughStatuses: [404] and map 404 to the same CLIError. Non-blocking; the command still errors cleanly today.
• Software engineering — incident.ts:126-129 (formatWhen). Guards null/empty but not an unparseable date string, so a malformed ISO value from the backend would render as Invalid Date. Cosmetic only; normalizeIncidentReport already keeps these as strings by design.
• Consistency (validated, not a gap) — analytics. Unlike metrics.ts/logs.ts, this command intentionally omits reportCliUsage('cli.diagnose.…'). That is the right call here: reportCliUsage POSTs to the instance's own oss_host (src/lib/skills.ts:202), and this command runs precisely when the instance is unreachable — the report would just burn the 3s abort timeout against a dead box. trackDiagnose (platform-side PostHog) is correctly retained. Flagging only so it reads as deliberate.

What's good

• normalizeIncidentReport is genuinely defensive: every rendered field has a fallback, malformed operation entries are filtered, non-finite numbers coerce to null/0, and it throws a clear CLIError only when the payload isn't a report — appropriate for a command that runs during abnormal backend/instance states.
• First diagnose subcommand to ship a unit test file (incident.test.ts, 7 tests) covering both the output contract and the normalization edge cases — coverage is stronger than its siblings.
• Correctly reuses the platform-login gating convention (rejects unlinked + FAKE_PROJECT_ID/--api-key), and shutdownAnalytics is idempotent so the catch+finally double-call is safe.

Security: No security-relevant changes — project_id comes from the user's own local config and is interpolated into a path to their own project; no new user input reaches SQL/shell, the bearer is redacted in debug output, and no secrets/PII are logged or added to output.

Performance: Single platform request, no loops or hot-path work; nothing of concern.

Verdict

approved (informational — zero Critical findings; the Suggestion and Information items are non-blocking). Human approval via the normal GitHub flow. Note the PR's own gating: merge only after backend #814 deploys, per the PR description.

[jwfing]

Summary
This PR adds insforge diagnose incident in a way that matches the stated feature intent and existing diagnose-command conventions, with no blocking issues found.

Requirements context
I based intent on the PR description plus the new README entry in README.md, because I did not find separate design/spec docs or linked requirements in the checkout. The intended behavior is: add a new cloud-side diagnose incident subcommand, require platform login rather than --api-key link mode, print either a human-readable incident report or raw JSON, and keep working when the instance itself is unreachable. I could not execute the test suite in this workspace because the dev dependency runner is not installed here (vitest: not found).

Findings

Critical
(none)

Suggestion

• src/commands/diagnose/incident.ts:172-214, src/commands/diagnose/incident.test.ts:1-109 — The tests only exercise the pure formatter/normalizer helpers. They do not cover command-level behavior such as rejecting FAKE_PROJECT_ID link mode, preserving raw server payload under --json, or confirming the subcommand is actually wired into diagnose. Those are the highest-risk regressions for this feature, so an integration-style command test would materially improve confidence.
• src/commands/diagnose/incident.ts:111-113 — formatWhen() treats any non-empty string as a date and will print Invalid Date if the backend returns malformed timestamps. Given this command is explicitly meant for degraded/unusual platform states and already normalizes the rest of the payload defensively, it would be safer to fall back to the original string or unknown when new Date(iso) is invalid.

Information

• src/commands/diagnose/incident.ts:181-201 — No security-relevant issues found. The command performs a single authenticated platform fetch, rejects API-key link mode as documented, and does not add any new secret handling or shell/SQL surfaces.
• src/commands/diagnose/incident.ts:121-169, src/commands/diagnose/incident.ts:191-196 — No performance-relevant issues found. The implementation is a single request plus linear-time normalization/formatting over a small response payload.

Verdict
approved

[jwfing]

Review: diagnose incident subcommand

Summary: A clean, well-tested cloud-side diagnose incident command whose IncidentReport interface is an exact mirror of the paired backend endpoint — no blocking issues; one worth-fixing label gap and a couple of cosmetic notes.

Requirements context

No incident-specific spec exists under docs/specs/ (contents: 2026-03-27-diagnose-command-design.md, 2026-03-27-diagnose-implementation-plan.md, 2026-04-17-db-migrations-command-design.md) — none matches this subcommand. Assessed against the PR description and the paired backend PR InsForge/insforge-cloud-backend#814 (GET /projects/v1/:id/diagnose/incident), which I read to verify the request/response contract.

I verified the CLI IncidentReport interface (src/commands/diagnose/incident.ts:12-32) field-for-field against the backend's response shape (src/services/incident.service.ts IncidentReport + res.json(report) in incident.controller.ts): project_status, operation_status, instance_type, reachable.{metrics_last_seen_at,metrics_reporting,database_connect}, down_since, memory_before_down_pct, memory_latest_pct, postgres_started_at, scrape_gaps_24h, recent_platform_operations[].{action,at}, verdict, explanation, recommendation — all match.

---

Critical

(none)

Suggestion

Functionality — one backend verdict is missing a human label. The backend defines six verdicts (incident.service.ts:102-108): paused_or_suspended, platform_operation_in_progress, oom_likely, down_unknown, metrics_stopped, no_incident_detected. The CLI's VERDICT_LABELS (src/commands/diagnose/incident.ts:36-42) maps only five — metrics_stopped is absent. That verdict is emitted on a real, common path (DB reachable but metrics stale — incident.service.ts:418-424), not a hypothetical future one. formatIncidentReport falls back to the raw string (VERDICT_LABELS[report.verdict] ?? report.verdict, incident.ts:118), so it degrades gracefully to Verdict: metrics_stopped rather than a polished label like "Metrics stopped reporting". Non-blocking (the explanation/facts/recommendation all still render), but since it ships alongside #814 it reads as an oversight rather than forward-compat. Adding the sixth label — and a test asserting every backend verdict maps — would close it. The existing falls back to the raw verdict string test (incident.test.ts:56-59) is aimed at genuinely-unknown future verdicts, so it doesn't catch this.

Information

• Empty recommendation prints a bare line. When the backend sends recommendation: '' (e.g. the no_incident_detected path, incident.service.ts:210), formatIncidentReport unconditionally pushes What to do: ${report.recommendation} (incident.ts:159), yielding a dangling What to do: with nothing after it. Consider skipping the line when the recommendation is empty.
• formatWhen output is locale/timezone-dependent. new Date(iso).toLocaleString() (incident.ts:132-135) renders in the host locale/TZ, and a malformed-but-non-empty date string surfaces as Invalid Date (the asNullableString guard only filters null/empty). Acceptable for a human-facing CLI, and the tests correctly avoid asserting the formatted date — just noting it.

---

Notes on things I checked and found correct

• reportCliUsage omission is deliberate and correct. Unlike sibling metrics/logs, this command intentionally does not call reportCliUsage — that helper reports to the instance (oss_host), which is exactly what's down during an incident. Keeping only trackDiagnose (platform-side PostHog) is the right call and matches the PR's stated "removed legacy telemetry".
• Error/analytics lifecycle matches convention. The try/catch(shutdownAnalytics → handleError)/finally(shutdownAnalytics) shape mirrors metrics.ts/logs.ts; shutdownAnalytics nulls its client so the double-call is idempotent, and handleError process.exits as elsewhere.
• Defensive normalization is thorough. normalizeIncidentReport coerces every rendered field, drops malformed operation entries, rejects non-report payloads with a clear CLIError, and the --json path passes the raw server payload through (forward-compatible with unknown fields). Good.
• Security: no security-relevant changes — no user-supplied input reaches SQL/shell/HTTP (the project_id in the path comes from local link config, same as siblings), no secrets logged, requireAuth + platform-login guard (FAKE_PROJECT_ID rejected) preserved, no new dependencies.
• Performance: no concerns — a single platformFetch with no loops, small payload, no blocking work.
• Tests/build: the 7 unit tests pass locally; tsc --noEmit produces no new errors in incident.ts/incident.test.ts (the pre-existing prompts.ts and other unrelated errors reproduce on main).

Verdict

approved — no Critical findings. The metrics_stopped label is a genuine (non-blocking) gap I'd encourage fixing before/with merge; the rest are optional polish. As the PR notes, land this only after #814 deploys, or the endpoint 404s. (Informational verdict — human approval via the normal flow.)

[jwfing]

LGTM - approved.