Skip to content

chore(cloud): post-merge /clean follow-up on lifecycle job code#7845

Merged
lalalune merged 1 commit into
developfrom
chore/cloud-lifecycle-post-clean
May 20, 2026
Merged

chore(cloud): post-merge /clean follow-up on lifecycle job code#7845
lalalune merged 1 commit into
developfrom
chore/cloud-lifecycle-post-clean

Conversation

@standujar
Copy link
Copy Markdown
Collaborator

@standujar standujar commented May 20, 2026

Five small wins surfaced by a second `/clean` pass after the lifecycle queue stack merged: #7810, #7813, #7815, #7816.

What changed (-5 LOC, 5 files)

`provisioning-jobs.ts`

  • Drop 4 redundant type casts. `status: "error"`, `"deletion_failed"`, and the `webhook_status` updates are all literals already matching the inferred parameter type — the `as Parameters[1]` / `as Partial` casts added nothing.
  • `executeAgentProvision` failure path now uses `agentProvisionJobResultToRecord({...})` like every other executor, preserving the typed-serialization-boundary pattern the file otherwise enforces consistently.
  • Drop 3 obvious comments (`// Store partial result for debugging`, `// Mark completed with result`, `// Fire webhook if configured`) to match the comment-free shape of the 6 sibling `executeAgent*` methods.

`v1/eliza/agents/[id]/route.ts` (DELETE)

  • Drop the redundant `instanceof Error && error.message === "Agent not found"` branch. `failureResponse()` already maps any "not found" error to 404 via downstream string match.

`v1/agents/[id]/logs/route.ts`

  • Add `success: false` to the 404 response body to match the response shape every sibling route uses.

`v1/eliza/agents/[id]/resume/route.ts`

  • Trim a forward-looking comment about a "future docker start fast path" — belongs in a ticket, not the route. Kept the audit-log rationale.

`tests/provisioning-job-types.test.ts`

  • Lock the registry size with `expect(Object.keys(JOB_TYPES)).toHaveLength(7)`. A new entry added to `JOB_TYPES` without a matching wire-value assertion now fails CI instead of being silently under-covered by the snake-case / uniqueness tests below.

Why now

The lifecycle stack landed in one big push at 02:09 UTC. Post-merge I ran a second `/clean` pass to surface anything that slipped through the per-PR reviews. These are the 5 high-confidence wins. Nothing behavioral — type cast removal compiles to the same JS; the response-shape fix in the logs 404 matches what `failureResponse` already emits elsewhere; the test additionnal assertion is purely a CI guardrail.

Test plan

  • `tsc --noEmit` clean on cloud-shared (touched files only)
  • `bun test packages/cloud-shared/src/lib/services/tests/provisioning-job-types.test.ts` → 4 pass, 17 assertions
  • biome check clean on all 5 touched files

Greptile Summary

Post-merge cleanup pass across five files in the lifecycle job stack: removes redundant type casts, normalizes the executeAgentProvision failure path to use the typed serializer, drops stale comments, and adds a CI guard on the JOB_TYPES registry size.

  • provisioning-jobs.ts: four unnecessary as Parameters<…>[1] / as Partial<Job> casts removed; the executeAgentProvision failure path now routes through agentProvisionJobResultToRecord for consistency with all other executors; three self-evident inline comments dropped.
  • route.ts (DELETE): the hand-rolled "Agent not found" → 404 branch is removed; failureResponse already reaches 404 via inferStatusFromLegacyError's message.includes("not found") check — the response now also carries code: "resource_not_found" which was previously absent.
  • logs/route.ts & resume/route.ts: 404 response body aligned with sibling routes (success: false added); forward-looking docker comment trimmed.

Confidence Score: 5/5

All five changes are non-behavioral: type cast removal compiles to identical JS, the response-shape additions are purely additive, and the test change only tightens an existing assertion.

Every change is either a TypeScript-only cast removal (zero JS delta), an additive response field (adds code: "resource_not_found" where it was absent before — no client would break on an extra field), a comment edit, or a test guardrail. The agentProvisionJobResultToRecord normalization on the failure path is functionally identical because the serializer is a plain object spread. tsc --noEmit and biome are reported clean across all touched files.

No files require special attention. The DELETE route behavior change (response now includes code) is worth confirming against any clients that strictly parse the 404 body, but it is purely additive.

Important Files Changed

Filename Overview
packages/cloud-shared/src/lib/services/provisioning-jobs.ts Removes 4 redundant type casts and normalizes the failure path in executeAgentProvision to use the typed agentProvisionJobResultToRecord serializer; drops 3 obvious inline comments. No behavioral change.
packages/cloud-api/v1/eliza/agents/[agentId]/route.ts Removes the redundant explicit "Agent not found" → 404 branch; failureResponse already maps error messages containing "not found" to 404 via inferStatusFromLegacyError. New response shape adds a code: "resource_not_found" field that was previously absent — additive only.
packages/cloud-api/v1/agents/[agentId]/logs/route.ts Adds success: false to the 404 response body for agent-not-found, aligning it with the shape every sibling route already emits.
packages/cloud-api/v1/eliza/agents/[agentId]/resume/route.ts Trims the forward-looking "future docker start fast path" clause from a block comment, keeping only the audit-log rationale that is currently true.
packages/cloud-shared/src/lib/services/tests/provisioning-job-types.test.ts Adds a toHaveLength(7) guard to the "includes every registered job type" test, making the registry size an explicit CI contract so new entries without matching assertions fail immediately.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[executeAgentProvision] --> B{provResult.success?}
    B -- No --> C[agentProvisionJobResultToRecord with error]
    C --> D[jobsRepository.update result]
    D --> E[throw Error]
    B -- Yes --> F[build AgentProvisionJobResult]
    F --> G[agentProvisionJobResultToRecord]
    G --> H[jobsRepository.updateStatus completed]
    H --> I{job.webhook_url?}
    I -- Yes --> J[fireWebhook]
    I -- No --> K[Done]
    J --> K
Loading

Reviews (1): Last reviewed commit: "chore(cloud): post-merge /clean follow-u..." | Re-trigger Greptile

Five small wins surfaced by a second /clean pass after the lifecycle
queue stack merged (#7810/#7813/#7815/#7816):

- provisioning-jobs.ts: drop 4 redundant type casts. `status: "error"`,
  `"deletion_failed"`, and the `webhook_status` updates are all literals
  matching the inferred parameter type — the `as Parameters<...>[1]` /
  `as Partial<Job>` casts added nothing.
- provisioning-jobs.ts: executeAgentProvision failure path now uses
  agentProvisionJobResultToRecord({...}) like every other executor,
  preserving the typed-serialization-boundary pattern.
- v1/eliza/agents/[id]/route.ts (DELETE): drop the redundant
  `instanceof Error && error.message === "Agent not found"` branch.
  failureResponse() already maps any "not found" error to 404.
- v1/agents/[id]/logs/route.ts: add `success: false` to the 404 body
  to match the response shape every sibling route uses.
- v1/eliza/agents/[id]/resume/route.ts: trim a forward-looking comment
  about a "future docker start fast path" — belongs in a ticket, not
  the route. Kept the audit-log rationale.
- __tests__/provisioning-job-types.test.ts: lock the registry size
  with `expect(Object.keys(JOB_TYPES)).toHaveLength(7)`. A new entry
  without a matching wire-value assertion now fails CI instead of being
  silently under-covered.

Net diff: -5 LOC, 5 files. No behavior change.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b0ac8a9f-f18d-4624-874e-97edd5faaad6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/cloud-lifecycle-post-clean

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

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

@github-actions github-actions Bot added the Tests label May 20, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 20, 2026

Claude encountered an error —— View job


I'll analyze this and get back to you.

@lalalune lalalune merged commit a997418 into develop May 20, 2026
40 of 42 checks passed
@lalalune lalalune deleted the chore/cloud-lifecycle-post-clean branch May 20, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants