Skip to content

refactor(services): extract agent-ownership predicate to shared helper (closes #4377)#4388

Merged
bokelley merged 1 commit intomainfrom
bokelley/extract-agent-ownership-helper
May 11, 2026
Merged

refactor(services): extract agent-ownership predicate to shared helper (closes #4377)#4388
bokelley merged 1 commit intomainfrom
bokelley/extract-agent-ownership-helper

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

Closes #4377. Extracts the agent-ownership JOIN predicate into `server/src/services/agent-ownership.ts` so the two distinct semantic uses share one source.

The drift surface

PR #4250's review flagged two inline copies of the same JOIN with subtly different semantics:

  • `registry-api.ts` — "find any org the user is a member of that owns this agent"
  • `member-tools.ts` — "verify THIS specific org is the agent's owner for this user"

Both query `member_profiles.agents @> [...] JOIN organization_memberships` but with different selectivity. Three call sites adding more inline copies is when rot sets in.

What changed

  • New `server/src/services/agent-ownership.ts` with two helpers:
    • `findOwnerOrgForUser(userId, agentUrl): Promise<string | null>`
    • `isOrgOwnerOfAgent(orgId, userId, agentUrl): Promise`
  • `registry-api.ts` — `resolveAgentOwnerOrg` is now a closure-scoped alias for `findOwnerOrgForUser`. Existing call sites unchanged.
  • `member-tools.ts` — inline JOIN replaced with `isOrgOwnerOfAgent(...)` call.
  • 7 unit tests covering happy path, no-match, error-on-throw, and the semantic distinction between the two helpers (parameter count + WHERE clause shape).

Note on active-membership filtering

`organization_memberships` has no status column in this schema — removed members get their row deleted, not status-flipped. Row existence is the membership signal. Documented in the new module's doc comment so future readers don't add a redundant `AND status = 'active'` filter.

Test plan

  • `npx vitest run tests/unit/agent-ownership.test.ts` — 7 passing
  • Existing integration tests around the two call sites still pass (route-level ownership gates, owner-test canonical-write)
  • No behavior change at any API surface — semantics preserved

🤖 Generated with Claude Code

closes #4377)

PR #4250's review flagged the agent-ownership JOIN as a drift surface
— duplicated inline in two places with subtly different semantics:

- registry-api.ts:4825 `resolveAgentOwnerOrg(userId, agentUrl)` —
  "any org the user is a member of that owns the agent"
- member-tools.ts:3588 (inline) — "the resolved member-context org IS
  the owning org" (tighter predicate that adds the org_id constraint)

Both queries join `member_profiles.agents` against `organization_
memberships` — same canonical relation, different selectivity.

This PR extracts both to `server/src/services/agent-ownership.ts`:

- `findOwnerOrgForUser(userId, agentUrl): Promise<string | null>` —
  registry-api.ts's "discover ownership" use case
- `isOrgOwnerOfAgent(orgId, userId, agentUrl): Promise<boolean>` —
  member-tools.ts's "confirm specific org" use case

Both call sites updated to use the helpers. `resolveAgentOwnerOrg` is
retained as a closure-scoped alias inside the registry-api factory so
existing call sites don't need to thread the import.

Note on active-membership filtering: `organization_memberships` has
no status column in this schema — removed members get their row
deleted, not status-flipped. Row existence is the membership signal.
Documented in the helper file's module doc.

Tests (7 passing):
- findOwnerOrgForUser returns org_id, null on no match, null on throw
- isOrgOwnerOfAgent returns true/false correctly
- semantic distinction pinned: findOwnerOrgForUser uses 2 params (no
  org filter); isOrgOwnerOfAgent uses 3 params (org_id constraint)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 9123097 into main May 11, 2026
14 checks passed
@bokelley bokelley deleted the bokelley/extract-agent-ownership-helper branch May 11, 2026 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: extract resolveAgentOwnerOrg to shared helper (drift surface)

1 participant