Skip to content

fix(api): support status query on company agent list#4450

Open
starlein wants to merge 1 commit intopaperclipai:masterfrom
starlein:fix/upstream-agents-status-query-filter
Open

fix(api): support status query on company agent list#4450
starlein wants to merge 1 commit intopaperclipai:masterfrom
starlein:fix/upstream-agents-status-query-filter

Conversation

@starlein
Copy link
Copy Markdown

Thinking Path

  • Paperclip orchestrates AI agents for company workflows where automation logic depends on predictable API contracts.
  • The agent-list API (GET /api/companies/:companyId/agents) is part of that control-plane contract used by skills/templates to find available assignees.
  • Many shipped templates/skills call this route with ?status=idle for auto-assignment and backlog triage flows.
  • The current route rejects all query parameters, returning 400 Unsupported query parameter: status, which breaks those workflows despite idle being a valid agent status.
  • This creates a mismatch: issue-list routes support status filtering, but agent-list does not, causing avoidable runtime failures in generated company routines.
  • This pull request adds explicit, validated status filtering support on the company agent-list route while keeping strict rejection for unknown query params.
  • The benefit is that existing templates/skills work as documented and operator workflows can reliably fetch idle/active agent subsets from the API.

What Changed

  • Added support for status query param on GET /api/companies/:companyId/agents.
  • Preserved strict query validation for unknown params (still returns 400 for unsupported keys).
  • Implemented status parsing for comma-separated values (e.g. status=idle,running) and repeated params.
  • Added validation against AGENT_STATUSES; invalid/empty status filters now return clear 400 errors.
  • When terminated is requested, the route now loads with includeTerminated: true and then applies status filtering.
  • Added/updated route tests in server/src/__tests__/agent-permissions-routes.test.ts for:
    • accepted status filtering,
    • terminated inclusion behavior,
    • invalid status rejection,
    • empty status rejection,
    • unsupported param rejection.

Verification

  • pnpm --filter @paperclipai/server typecheck
  • pnpm --filter @paperclipai/server exec vitest run src/__tests__/agent-permissions-routes.test.ts

Risks

  • Low risk: behavior change is scoped to one GET route and guarded by explicit validation.
  • Potential compatibility note: requests like ?status= now fail fast with 400 instead of silently returning an unfiltered list.

For core feature work, check ROADMAP.md first and discuss it in #dev before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See CONTRIBUTING.md.

Model Used

  • OpenAI gpt-5.3-codex via Hermes Agent (tool-assisted code editing, tests, git/gh workflow).

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots
  • I have updated relevant documentation to reflect my changes
  • I have considered and documented any risks above
  • I will address all Greptile and reviewer comments before requesting merge

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 25, 2026

Greptile Summary

This PR adds validated status query-parameter filtering to GET /api/companies/:companyId/agents, supporting comma-separated values, repeated params, and proper includeTerminated passthrough when terminated is requested. The implementation is clean and the new tests cover the key cases well.

Confidence Score: 5/5

Safe to merge — logic is correct and well-tested; two minor P2 follow-ups are non-blocking.

All findings are P2: one stale doc line and one defensive note about the implicit includeTerminated: false default. Neither blocks correctness of the new behaviour. Tests are thorough and the route change is well-scoped.

docs/api/agents.md — line 16 contradicts the new status-filter feature and should be updated before consumers rely on it.

Important Files Changed

Filename Overview
server/src/routes/agents.ts Adds status query param support with comma-separated parsing, AGENT_STATUSES validation, and includeTerminated passthrough; minor concern around always passing includeTerminated: false where the original passed no second arg.
server/src/tests/agent-permissions-routes.test.ts Adds five well-scoped tests covering happy-path filtering, terminated inclusion, invalid/empty status rejection, and unsupported-param rejection; all test cases are clear and targeted.

Comments Outside Diff (1)

  1. docs/api/agents.md, line 16 (link)

    P2 Stale documentation contradicts new behavior

    Line 16 still states "This route does not accept query filters. Unsupported query parameters return 400." — but this PR explicitly adds status filter support. Any agent, skill, or external consumer reading this doc will believe status filtering is unsupported and won't use the new feature.

    Context Used: docs/api/agents.md (source)

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: docs/api/agents.md
    Line: 16
    
    Comment:
    **Stale documentation contradicts new behavior**
    
    Line 16 still states "This route does not accept query filters. Unsupported query parameters return `400`." — but this PR explicitly adds `status` filter support. Any agent, skill, or external consumer reading this doc will believe status filtering is unsupported and won't use the new feature.
    
    
    
    **Context Used:** docs/api/agents.md ([source](https://app.greptile.com/review/custom-context?memory=b2b95ce1-5efc-4c63-95ac-60273119c8d2))
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: docs/api/agents.md
Line: 16

Comment:
**Stale documentation contradicts new behavior**

Line 16 still states "This route does not accept query filters. Unsupported query parameters return `400`." — but this PR explicitly adds `status` filter support. Any agent, skill, or external consumer reading this doc will believe status filtering is unsupported and won't use the new feature.

```suggestion
Supports an optional `status` query filter. Pass one or more comma-separated valid statuses (e.g. `?status=idle,running`). Unsupported query parameters return `400`.
```

**Context Used:** docs/api/agents.md ([source](https://app.greptile.com/review/custom-context?memory=b2b95ce1-5efc-4c63-95ac-60273119c8d2))

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1162

Comment:
**`includeTerminated` always passed even with no status filter**

The original call was `svc.list(companyId)` (no second argument). Now every request — including ones with no `status` param — calls `svc.list(companyId, { includeTerminated: false })`. If the service method's default behaviour when the option is omitted differs from `{ includeTerminated: false }` (e.g. it previously returned terminated agents by default), this silently changes the unfiltered list response for all existing callers. Worth a quick verification against the `AgentService.list` signature to confirm `includeTerminated` defaults to `false`.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(api): support status query on compan..." | Re-trigger Greptile


const statusFilter = new Set(requestedStatuses);
const includeTerminated = statusFilter.has("terminated");
const result = await svc.list(companyId, { includeTerminated });
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 includeTerminated always passed even with no status filter

The original call was svc.list(companyId) (no second argument). Now every request — including ones with no status param — calls svc.list(companyId, { includeTerminated: false }). If the service method's default behaviour when the option is omitted differs from { includeTerminated: false } (e.g. it previously returned terminated agents by default), this silently changes the unfiltered list response for all existing callers. Worth a quick verification against the AgentService.list signature to confirm includeTerminated defaults to false.

Prompt To Fix With AI
This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 1162

Comment:
**`includeTerminated` always passed even with no status filter**

The original call was `svc.list(companyId)` (no second argument). Now every request — including ones with no `status` param — calls `svc.list(companyId, { includeTerminated: false })`. If the service method's default behaviour when the option is omitted differs from `{ includeTerminated: false }` (e.g. it previously returned terminated agents by default), this silently changes the unfiltered list response for all existing callers. Worth a quick verification against the `AgentService.list` signature to confirm `includeTerminated` defaults to `false`.

How can I resolve this? If you propose a fix, please make it concise.

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.

1 participant