fix(compute): address PR review — validation, cleanup, version bump

[tonychang04]

Summary

• Remove dead variable in getFlyToken (deploy.ts)
• Wrap cleanup in try-catch so deploy errors aren't masked (deploy.ts)
• Fix reportCliUsage skipped on empty-list and no-logs paths (list.ts, logs.ts)
• Fix -MB placeholder showing when memory is null (list.ts)
• Validate --limit as positive integer, clamp to [1, 1000] (logs.ts)
• Validate --port and --memory as finite numbers (update.ts)
• Fix flaky test: accept creating/deploying status, not just running (compute.test.ts)
• Fix fragile assertion: check specific service ID, not all cli-test-* (compute.test.ts)
• Bump version to 0.1.42

🤖 Generated with Claude Code

Note

Fix validation, cleanup error handling, and usage tracking in compute commands

• Wraps unlinkSync/renameSync cleanup in a try/catch in registerComputeDeployCommand so filesystem errors during teardown no longer mask the real deploy result.
• Adds input validation for --port and --memory in registerComputeUpdateCommand, throwing a CLIError for non-numeric values instead of silently coercing to NaN.
• Clamps the --limit option in registerComputeLogsCommand to the range [1, 1000].
• Records usage success before early return when no services or logs are found in non-JSON mode for list and logs commands.
• Fixes memory display in list to show '-' for falsy values (e.g. 0) instead of '0MB'.

^{Macroscope summarized afd595b.}

Summary by CodeRabbit

Release Notes

• New Features

  • Added input validation for --port and --memory options in compute update command with descriptive error messages.

• Bug Fixes

  • Fixed memory display in compute list command to show "-" instead of "-MB" when unavailable.
  • Improved deploy command cleanup to prevent rollback errors from masking primary deployment failures.
  • Added bounds checking for logs command --limit option (enforced range: 1-1000).

• Chores

  • Bumped package version to 0.1.42.

[coderabbitai]

Walkthrough

Version bump to 0.1.42 with hardened error handling in deploy cleanup, numeric validation for port/memory options, improved memory formatting, clamped log limits, and better CLI usage reporting. Integration tests updated for status flexibility.

Changes

Cohort / File(s)	Summary
Package Metadata package.json	Version bumped from 0.1.41 to 0.1.42.
Deploy Hardening src/commands/compute/deploy.ts	Removed getProjectConfig() dependency in getFlyToken(); now requires only process.env.FLY_API_TOKEN. Wrapped fly.toml restoration logic in cleanup phase with try/catch to prevent cleanup failures from masking primary deploy errors.
Compute Commands Enhancement src/commands/compute/list.ts, src/commands/compute/logs.ts, src/commands/compute/update.ts	Added CLI usage reporting on empty results (list, logs). Fixed memory cell formatting to avoid "-MB" fallback. Clamped log --limit to bounded range (1–1000). Added explicit numeric validation for --port and --memory with Number.isFinite() checks.
Integration Tests src/integration/compute.test.ts	Tracked deleted service ID; relaxed startup status assertions to accept multiple non-terminal states (running, creating, deploying). Updated post-delete verification to check by id instead of name prefix.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat(compute): add compute service CLI commands #52: Introduces the compute command features (deploy, list, logs, update) that are being hardened and improved in this PR.
• release 0.1.34 #37: Prior version bump of the same package, modifying package.json version field.

Suggested reviewers

• jwfing
• Fermionic-Lyu

Poem

🐰 With tokens free and limits bound,
Our ports and memory safe and sound,
Deploy's repairs now catch the fall,
No hidden errors mask it all—
Version forty-two does call! 🎉

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(compute): address PR review — validation, cleanup, version bump' directly addresses the main changes across the compute commands, capturing validation improvements, error handling cleanup, and the version bump.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/compute-review-feedback

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/integration/compute.test.ts (1)

64-67: Consider relaxing the status assertion here as well.

The "compute get" test (line 66) still expects exactly 'running', but if test execution is fast, the service may still be in creating or deploying state. This could cause intermittent flakiness similar to what was addressed in the "compute create" test.

♻️ Proposed fix

     expect(payload.id).toBe(createdServiceId);
-    expect(payload.status).toBe('running');
+    expect(['running', 'creating', 'deploying']).toContain(payload.status);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/integration/compute.test.ts` around lines 64 - 67, The test currently
asserts payload.status is exactly 'running' after the "compute get" step; relax
this by allowing intermediate states so the test isn't flaky: update the
assertion that references payload.status (in the "compute get" test that
compares payload.id to createdServiceId) to accept 'creating', 'deploying', or
'running' (e.g., replace the strict toBe('running') check with a containment or
regex check that allows those three statuses).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/integration/compute.test.ts`:
- Around line 64-67: The test currently asserts payload.status is exactly
'running' after the "compute get" step; relax this by allowing intermediate
states so the test isn't flaky: update the assertion that references
payload.status (in the "compute get" test that compares payload.id to
createdServiceId) to accept 'creating', 'deploying', or 'running' (e.g., replace
the strict toBe('running') check with a containment or regex check that allows
those three statuses).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5029d0ec-81f4-4db0-9147-e14f87b7ac4d

📥 Commits

Reviewing files that changed from the base of the PR and between 062fe8a and afd595b.

📒 Files selected for processing (6)

• package.json
• src/commands/compute/deploy.ts
• src/commands/compute/list.ts
• src/commands/compute/logs.ts
• src/commands/compute/update.ts
• src/integration/compute.test.ts