insta logs --deploy (compute deploy events) - #64
Conversation
jwfing
left a comment
There was a problem hiding this comment.
Review: insta logs --deploy (compute deploy events)
Summary: Cleanly adds a --deploy flag to insta logs that routes compute requests to the new platform /deploy-events endpoint, extracting two pure helpers (deployEventsPath, deployEventLine) with unit tests — well-scoped and consistent with the existing metrics.ts patterns.
Requirements context: No matching spec/plan found — this repo has no docs/superpowers/ or docs/specs/ directory, so I assessed against the PR description (add --deploy → /deploy-events, compute-only, filters, --json) and the AGENTS.md / developing-insta-cli conventions.
Critical
(none) — no correctness, security, or data-loss issues found.
Suggestion
-
Functionality —
--regionis silently dropped under--deploy(src/commands/metrics.ts:82-84,98; flag declared atsrc/index.ts:177). Thelogscommand still advertises--option '--region <r>', and the runtime path forwards it (metrics.ts:105), butdeployEventsPathhas noregionparam, soinsta logs compute --deploy --region xxxaccepts the flag and ignores it with no feedback. Either forward it to/deploy-events(if the endpoint supports it) or emit a short note that--regiondoesn't apply to deploy events, so the behavior isn't silently surprising. -
Software engineering / functionality — response-shape contract is assumed but untested (
src/commands/metrics.ts:100-102,87-90). The new tests cover only the two pure helpers (path building + line rendering), matching the repo's "extract pure fns, test those" convention. But the deploy branch assumes the platform returns{ note, events: [{ ts, origin, type, status, instance }] }. Since these field names aren't exercised by any test, a mismatch with the platform PR's actual/deploy-eventspayload would render blank/partial lines (ts undefined, empty[]) and print(no deploy events)— all without failing CI. Worth confirming the field names against the platform PR before merge; thets ?? ''/origin ?? ''fallbacks mean failures degrade silently rather than erroring.
Information
-
Convention — mirror the flag in the CLI reference doc.
AGENTS.mdand thedeveloping-insta-cliskill both state command/flag changes "must be mirrored inskills/insta/cli-reference.md" (superproject submodule) in the same change set. That file lives outside this repo so I can't verify it here — just flagging that the--deployflag is only half-shipped until the reference doc is updated. -
Functionality — all-empty event renders a bare line (
src/commands/metrics.ts:88-89). An event with every field undefined produces" [] : ". Acceptable given the platform returns structured events, but noting it.
Dimension coverage
- Security: No new attack surface. User-supplied
group/branch/limit/instanceflow throughURLSearchParams(qs,metrics.ts:4-9) and are properly encoded;projectIdis interpolated into the path exactly as the pre-existinglogs/metrics/usagepaths already do (sourced from linked project config, not free user input). No secrets or tokens are logged or newly returned. - Performance: No concern — a single GET plus a linear render of the returned events. No N+1, no blocking work added.
Verdict: approved (informational — zero Critical findings). Two suggestions worth a look before merge (silent --region, and confirming the /deploy-events payload field names), but nothing blocking. The GitHub green-check approval remains a separate human action via the maintainer approve flow.
There was a problem hiding this comment.
2 issues found across 3 files
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/metrics.ts">
<violation number="1" location="src/commands/metrics.ts:93">
P3: Agent-facing command references remain stale: README omits `--deploy`, and the required `skills/insta/cli-reference.md` update is absent from this change. Document the new flag and its compute-only deploy-event behavior with the implementation.</violation>
<violation number="2" location="src/commands/metrics.ts:96">
P3: When `--deploy` is used, the `--region` option is silently ignored: deploy-events don't support region filtering, but the option is still accepted without any feedback to the user. Someone familiar with runtime logs might pass `--region` expecting it to narrow deploy events too, and get no indication that it was dropped. Consider either warning when `--deploy` and `--region` are combined, or omitting `--region` from help text when deploy mode is active (though commander doesn't support dynamic options easily, so a warning in the handler is the practical path).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| // insta logs <db|compute> [group] | ||
| export async function logs(component: string, group: string | undefined, opts: { branch?: string; limit?: string; region?: string; instance?: string; json?: boolean }): Promise<void> { | ||
| export async function logs(component: string, group: string | undefined, opts: { branch?: string; limit?: string; region?: string; instance?: string; json?: boolean; deploy?: boolean }): Promise<void> { |
There was a problem hiding this comment.
P3: Agent-facing command references remain stale: README omits --deploy, and the required skills/insta/cli-reference.md update is absent from this change. Document the new flag and its compute-only deploy-event behavior with the implementation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/metrics.ts, line 93:
<comment>Agent-facing command references remain stale: README omits `--deploy`, and the required `skills/insta/cli-reference.md` update is absent from this change. Document the new flag and its compute-only deploy-event behavior with the implementation.</comment>
<file context>
@@ -78,10 +78,30 @@ export async function usage(opts: { from?: string; to?: string; json?: boolean;
+
// insta logs <db|compute> [group]
-export async function logs(component: string, group: string | undefined, opts: { branch?: string; limit?: string; region?: string; instance?: string; json?: boolean }): Promise<void> {
+export async function logs(component: string, group: string | undefined, opts: { branch?: string; limit?: string; region?: string; instance?: string; json?: boolean; deploy?: boolean }): Promise<void> {
const api = await ApiClient.load()
const p = await requireProject()
</file context>
| if (opts.deploy) { | ||
| if (component !== 'compute') return info('deploy events are only available for compute') |
There was a problem hiding this comment.
P3: When --deploy is used, the --region option is silently ignored: deploy-events don't support region filtering, but the option is still accepted without any feedback to the user. Someone familiar with runtime logs might pass --region expecting it to narrow deploy events too, and get no indication that it was dropped. Consider either warning when --deploy and --region are combined, or omitting --region from help text when deploy mode is active (though commander doesn't support dynamic options easily, so a warning in the handler is the practical path).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/metrics.ts, line 96:
<comment>When `--deploy` is used, the `--region` option is silently ignored: deploy-events don't support region filtering, but the option is still accepted without any feedback to the user. Someone familiar with runtime logs might pass `--region` expecting it to narrow deploy events too, and get no indication that it was dropped. Consider either warning when `--deploy` and `--region` are combined, or omitting `--region` from help text when deploy mode is active (though commander doesn't support dynamic options easily, so a warning in the handler is the practical path).</comment>
<file context>
@@ -78,10 +78,30 @@ export async function usage(opts: { from?: string; to?: string; json?: boolean;
+export async function logs(component: string, group: string | undefined, opts: { branch?: string; limit?: string; region?: string; instance?: string; json?: boolean; deploy?: boolean }): Promise<void> {
const api = await ApiClient.load()
const p = await requireProject()
+ if (opts.deploy) {
+ if (component !== 'compute') return info('deploy events are only available for compute')
+ const res = await api.request('GET', deployEventsPath(p.projectId, { group, branch: opts.branch ?? p.branch, limit: opts.limit, instance: opts.instance }))
</file context>
| if (opts.deploy) { | |
| if (component !== 'compute') return info('deploy events are only available for compute') | |
| if (opts.deploy) { | |
| if (opts.region) info('note: --region is not applicable to deploy events, ignoring') | |
| if (component !== 'compute') return info('deploy events are only available for compute') |
Adds --deploy to
insta logs, hitting the new platform /deploy-events endpoint. Depends on the platform PR.🤖 Generated with Claude Code
Summary by cubic
Adds
--deploytoinsta logsto show compute deploy events (machine lifecycle). Uses the platform/deploy-eventsendpoint, with filtering and JSON output.insta logs compute --deployfetches from/projects/:id/deploy-events.--branch,--limit,--instance;--jsonreturns raw payload.Written for commit 6fcfd0b. Summary will update on new commits.