fix(deploy): existing-persona lookup uses /deployments list (not phantom /agents)#120
Conversation
…tom /agents)
`findExistingAgent` was calling
`GET /workspaces/{ws}/agents?persona_slug=<slug>`. That route is a
dashboard proxy to an external gateway: it requires session-cookie auth
and 403s for the `cli:auth` Bearer tokens the CLI uses. The actual list
of deployed personas — the `agents` table reader added in cloud#580 —
lives at `GET /workspaces/{ws}/deployments`, accepts `cli:auth`, and
returns `{agents:[{agentId, personaId, deployedName, status, ...}]}`.
Why no `?personaId=` server-side filter: `agents.personaId` on the
cloud schema is a UUID FK to `personas.id`. The CLI only knows the
persona's slug (the local persona JSON's `id` field). Sending the slug
as `personaId=` makes cloud's drizzle predicate throw on the UUID cast
→ 500. The list is workspace-scoped (dozens of rows, not thousands),
so the right tradeoff is fetching unfiltered and matching client-side
against `deployedName` (which cloud derives from
`persona.slug || persona.name || persona.id`), `personaSlug`, or
`personaId` as a fallback.
Changes:
* `findExistingAgent` → call `/deployments` (no query string).
* `parseAgentLike` accepts both `{agentId}` (new) and `{id}` (legacy
preview shape). When `expectedPersonaId` is supplied, match it
against any persona-identifying field on the row (deployedName,
personaSlug, personaId). Rows with NO persona info at all (legacy
preview shape that pre-filtered server-side) still pass through.
* Treat `status === 'destroyed'` as "not present" so a re-deploy with
the same slug doesn't trip on-exists against a tombstone.
* Rewrite test mocks to disambiguate the listing GET from the deploy
POST via `init.method` (both now share the same URL).
* Added two new tests: full /deployments shape parsing (with
tombstone + wrong-persona rows skipped), and ordering guarantee
(listing GET fires before deploy POST).
Smoke against production cloud with the user's actual Anthropic-
connected workspace: no more 403 on the existing-persona check, no
more 500 on the personaId filter. Bundle upload now reaches cloud's
deploy handler. (Next failure: cloud Lambda missing
`@parcel/watcher-linux-x64-glibc` native binary — server packaging
issue, separate PR.)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR migrates cloud deployment's "existing agent" detection from a persona-slug query parameter endpoint to the workspace ChangesCloud Deploy Mode Endpoint Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Addressing review findings on PR #120: * Workspace-scoped list rows without persona-identifying fields are no longer treated as matches. The "no persona fields → pass through" fallback was justified by legacy `{agent: {...}}` envelopes where the URL path implied persona-scoping, but cloud's new `/deployments` list is workspace-scoped — a row missing `deployedName`/`personaSlug`/ `personaId` could belong to any persona in the workspace, and acting on it would let on-exists destroy the wrong agent. The legacy envelope keeps its back-compat via a `requirePersonaMatch: false` opt-in; every array element from the new endpoint sets `requirePersonaMatch: true`. * When multiple rows match (destroy+redeploy race window, soft-delete windows), prefer the newest `active` row instead of whichever cloud returns first. Sort by status tier (active first) then `createdAt` descending. Previously the first parsed match won, which on most SQL backends meant insertion order — the opposite of what users expect. Tests: * `workspace-scoped list rows without persona-identifying fields are NOT matched` — proves the tightened filter ignores ambiguous rows. * `multiple active rows for the same persona — newest wins` — proves the newest-active tiebreaker. * `active row wins over an older active and over inactive rows` — proves status tier outranks createdAt. * `malformed array entries (null/empty) are skipped without throwing` — defensive parser regression guard. 101/101 deploy tests pass. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Fresh-eyes review feedback addressed in 9bdae07:
Four new tests:
101/101 deploy tests pass. The review's lower-priority follow-ups (pagination, |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/deploy/src/modes/cloud.test.ts`:
- Around line 684-686: The fetch mock in the test (the fetch(url, init) stub)
returns the old legacy envelope { agent: { id: 'agent-old' } } for GET
/deployments; update this mock to return the new workspace-scoped list shape so
the cancel-path test exercises persona matching: return { agents: [{ agentId:
'agent-old', deployedName: '<name>', status: '<status>' }] } (use realistic
deployedName/status values used elsewhere in the test) so later ambiguity
assertions are valid, or alternatively move the legacy-envelope assertion into a
dedicated parser compatibility test; adjust any downstream expectations in this
test that read agent.id to instead read agents[0].agentId (or adapt to helpers
that parse the new shape).
🪄 Autofix (Beta)
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: edb640be-245d-43a5-a3d4-9b52e69c208f
📒 Files selected for processing (2)
packages/deploy/src/modes/cloud.test.tspackages/deploy/src/modes/cloud.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/deploy/src/modes/cloud.ts
1 participant
Summary
findExistingAgentwas callingGET /workspaces/{ws}/agents?persona_slug=<slug>. That route is a dashboard proxy to an external gateway — it requires session-cookie auth and 403s for thecli:authBearer tokens the CLI uses. The real persona-deploy listing endpoint, added in cloud#580, isGET /workspaces/{ws}/deployments. Acceptscli:auth, returns{agents:[{agentId, personaId, deployedName, status, ...}], nextCursor}.Why no
?personaId=server-side filteragents.personaIdon the cloud schema is a UUID FK topersonas.id. The CLI only knows the persona's slug (the local persona JSON'sidfield, e.g.notion-essay-pr). Sending the slug aspersonaId=makes cloud's drizzle predicateeq(agents.personaId, slug)throw on the UUID cast → 500. The list is workspace-scoped (dozens of rows in practice, not thousands), so the right tradeoff is fetching unfiltered and matching client-side againstdeployedName(which cloud derives frompersona.slug || persona.name || persona.id), withpersonaSlugandpersonaIdas fallbacks.Filed as a follow-up: cloud's
/deployments?personaId=filter should accept slugs too (currently UUID-only → 500 on slug input).Changes
findExistingAgent: switched to/deployments(no query string), updated 401 hint to referenceagentworkforce login(not the legacyworkforce login).parseAgentLike: accepts both{agentId}(new) and{id}(legacy preview). WhenexpectedPersonaIdis supplied, matches against any persona-identifying field on the row (deployedName, personaSlug, personaId); rows with no persona info pass through (legacy preview shape that pre-filtered server-side).status === 'destroyed'rows so a re-deploy with the same slug doesn't trip on-exists against a tombstone.Tests
88 deploy tests pass. Updates:
init.method(both share the same URL now).{agents:[{agentId, personaId, deployedName, status}]}with destroyed tombstones and wrong-persona rows correctly skipped, only the active match returned.Smoke
Built locally and ran against
https://agentrelay.com/cloudwith the actual user workspace50587328-...:Next failure surfaces server-side: cloud's Lambda bundle is missing
@parcel/watcher-linux-x64-glibc—@agentworkforce/persona-kitcan't load on cloud → 500persona_kit_unavailable. That's cloud packaging; separate PR.Test plan
pnpm -F @agentworkforce/deploy run lintcleanpnpm -F @agentworkforce/deploy run test— 88/88 passagentrelay.com/cloudwith real workspace — existing-persona check passes, bundle upload reaches the deploy handler🤖 Generated with Claude Code