Repository navigation
Conversation
`insta compute status` showed only desired vs live, so a failed deploy read as `live=none` with no reason (SUP2-177). The platform now sends `lastFailure` on /services/:id/state while the newest attempt is a failure; print it, plus the `insta build logs` command when it was a build. Refs InsForge/instacloud-platform#646 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/commands/compute.ts">
<violation number="1" location="src/commands/compute.ts:428">
P3: The build-logs hint silently assumes --source github when the platform omits buildSource, but the build logs command itself defaults to --source archive (src/index.ts:545), and archive sources take deploy-operation IDs while github sources take GitHub build IDs. If the platform ever sends buildId without buildSource (the type here allows exactly that combination), a user debugging a failed archive/source-dir deploy would be handed 'insta build logs <op-id> --source github', which reads the wrong record. Since the companion platform work defines this contract, either guarantee buildSource whenever buildId is present and drop the github fallback, or only emit the hint when the source is known.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Turn on auto-fix | Re-trigger cubic
| if (!f) return lines | ||
| const at = Number.isNaN(Date.parse(f.at)) ? f.at : `${new Date(f.at).toISOString().slice(0, 16).replace('T', ' ')} UTC` | ||
| lines.push(`last ${f.kind} failed ${at}: ${f.reason}`) | ||
| if (f.buildId) lines.push(` → insta build logs ${f.buildId} --source ${f.buildSource ?? 'github'}`) |
There was a problem hiding this comment.
P3: The build-logs hint silently assumes --source github when the platform omits buildSource, but the build logs command itself defaults to --source archive (src/index.ts:545), and archive sources take deploy-operation IDs while github sources take GitHub build IDs. If the platform ever sends buildId without buildSource (the type here allows exactly that combination), a user debugging a failed archive/source-dir deploy would be handed 'insta build logs <op-id> --source github', which reads the wrong record. Since the companion platform work defines this contract, either guarantee buildSource whenever buildId is present and drop the github fallback, or only emit the hint when the source is known.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/commands/compute.ts, line 428:
<comment>The build-logs hint silently assumes --source github when the platform omits buildSource, but the build logs command itself defaults to --source archive (src/index.ts:545), and archive sources take deploy-operation IDs while github sources take GitHub build IDs. If the platform ever sends buildId without buildSource (the type here allows exactly that combination), a user debugging a failed archive/source-dir deploy would be handed 'insta build logs <op-id> --source github', which reads the wrong record. Since the companion platform work defines this contract, either guarantee buildSource whenever buildId is present and drop the github fallback, or only emit the hint when the source is known.</comment>
<file context>
@@ -411,7 +411,22 @@ export async function computeStatus(serviceName: string | undefined, opts: LifeO
+ if (!f) return lines
+ const at = Number.isNaN(Date.parse(f.at)) ? f.at : `${new Date(f.at).toISOString().slice(0, 16).replace('T', ' ')} UTC`
+ lines.push(`last ${f.kind} failed ${at}: ${f.reason}`)
+ if (f.buildId) lines.push(` → insta build logs ${f.buildId} --source ${f.buildSource ?? 'github'}`)
+ return lines
}
</file context>
jwfing
left a comment
There was a problem hiding this comment.
Summary
The change implements the requested failure display and preserves JSON output and compatibility with older platforms. No blocking correctness, security, or performance issues were found. Review used GitHub files at the supplied commit because the local shell could not start.
Requirements context
Reviewed platform issue #646, companion platform PR #647, the CLI README, supplied AGENTS.md, and the repository development skill. The requirements specify tenant-safe failure reasons, build-log hints, and server-side suppression after a later successful deploy. The CLI implementation matches that division of responsibility; no additional feature-specific requirements were found in the reviewed repository documentation.
Findings
Critical
(none)
Suggestion
- test/compute-status.test.ts: Cover the defensive fallback branches - The four tests cover the principal rendering cases. Add cases for an invalid timestamp and a buildId with omitted buildSource to protect the raw timestamp fallback and default github hint.
Information
- Functionality: Output matches the platform contract - The failure fields match the companion platform response. GitHub and archive hints use sources accepted by buildLogs; image failures omit the hint. The JSON early return remains unchanged, and absent lastFailure retains the original status line.
- Software engineering: Rendering follows existing conventions - The pure string-array helper and info loop match existing rendering patterns. Tests use Vitest and .js import specifiers consistently with the project's ESM style.
- Security: No new execution or authorization path - The added code only prints response fields; the suggested command is not executed. Authentication and request paths are unchanged, and no dependencies are added. Tenant-safe reason curation remains the platform's responsibility under the linked contract.
- Performance: Negligible rendering overhead - The change adds no API calls and produces at most three lines with bounded date-formatting work. No new unbounded loops, blocking I/O, or substantial allocations were identified.
Tests
Could not run typecheck or tests: shell execution failed before startup with a bwrap namespace-permission error. Inspected the four added tests and surrounding code through GitHub at d6a0939; these match the supplied diff. Local checkout cross-checking and the reported 3934-test result could not be independently verified.
Verdict
approved: No Critical findings were identified; the non-blocking test suggestion permits approval under the specified verdict rule.
Refs InsForge/instacloud-platform#646 (platform side: InsForge/instacloud-platform#647).
insta compute statusnow prints the reason when the newest deploy attempt failed. When the failure came from a build, it also prints the command to read that build's logs:--jsonpasses the platform response through unchanged. An older platform that doesn't sendlastFailuregets the same output as before.Tests:
test/compute-status.test.ts(new); full suite passes with 3934 tests.🤖 Generated with Claude Code
Summary by cubic
insta compute statusnow prints the reason the newest deploy attempt failed instead of leavinglive=noneunexplained, and shows theinsta build logscommand when the failure came from a build.--jsonoutput passes the platform response through unchanged, and older platforms that don't sendlastFailureget the same output as before.Written for commit d6a0939. Summary will update on new commits.