fix(activity): filter by action, reject unknown activity query params (BLO-21979)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work.
• Its activity/audit feed (GET /companies/:id/activity) is the primary tool an agent or operator uses to verify whether a given event actually happened.
• The route accepted ?action= but silently dropped it, so a caller filtering by action got a plausible, well-formed page of other actions.
• On an audit surface, “I could not verify” silently became “I verified it is absent,” which is the wrong failure mode.
• This pull request wires action through to the query, rejects unsupported query keys on this route and its GET siblings, and echoes the filter that was actually applied.
• The benefit is that callers now get the correct filtered result or an explicit unsupported-parameter error, never a silent no-op.

Linked Issues or Issue Description

Paperclip issue BLO-21979 describes this audit-feed bug. No GitHub issue exists, so the bug is described inline in the required issue-template shape.

What happened

GET /companies/:companyId/activity?limit=5&action=issue_write_denied returned 200 with unfiltered activity rows, even though a matching issue_write_denied row existed.

Expected behavior

The activity endpoint should either return only rows matching action=issue_write_denied or reject the request if the filter is unsupported. It must not silently ignore a filter on an audit surface.

Steps to reproduce

1. Create or identify a company with an issue_write_denied activity row.
2. Request GET /companies/:companyId/activity?limit=5&action=issue_write_denied.
3. Observe that the old route returned unrelated activity rows because the query parameter was ignored.

Paperclip version or commit

Observed before this PR on the branch that did not include activityService.list({ action }) support.

Deployment mode

Server/API route behavior; reproduced against the Paperclip deployment API surface.

What Changed

• server/src/services/activity.ts: added action to ActivityFilters and to the list query conditions.
• server/src/routes/activity.ts: passes action through on company activity requests and rejects unsupported query parameters on all four GET routes in the file.
• server/src/routes/activity.ts: adds X-Applied-Filters on company activity responses so callers can distinguish applied filters from empty results.
• server/src/routes/openapi.ts: documents the supported query parameters and 400 responses.
• Tests cover route-level action filtering, unsupported query parameter rejection, and embedded-Postgres service filtering.

Sibling-route audit

Checked nearby read/audit routes for the same accept-and-ignore pattern. The activity GET siblings now reject unsupported query parameters. cases.ts and pipelines.ts already validate their query shapes. companies.ts timeline/artifact query schemas intentionally passthrough. file-resources.ts has a related broader-default issue in a different subsystem and should be tracked separately.

Verification

• npx vitest run server/src/__tests__/activity-routes.test.ts server/src/__tests__/activity-service.test.ts passed locally: 18/18.
• npx vitest run server/src/__tests__/openapi-routes.test.ts passed locally: 3/3.
• tsc --noEmit on server/ completed cleanly.
• Manual repro now returns only matching activity rows, or a 400 when the query key is unsupported.

Risks

Low risk. The action filter is additive. The unknown-query-param rejection is a behavior change for callers sending stray keys to these audit routes; known UI callers send only supported keys. The response body remains the same array shape, and the applied-filter signal is header-only.

Model Used

Claude Sonnet 5 (claude-sonnet-5[1m]), agentic coding session with file inspection, route/service edits, and local test execution.

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 searched GitHub for duplicate or related PRs and linked them above
• I have either linked existing issues or described the issue in-PR
• I have run tests locally and they pass
• I have added or updated tests where applicable
• I have considered and documented risks above

[allyblockcast]

🔗 Paperclip issue: BLO-21979

[allyblockcast]

🔗 Paperclip issue: BLO-21979

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• No linked issue or inline issue description found — either tag an existing issue with Fixes #NNN / Closes #NNN / Refs #NNN, or describe the underlying issue inline in the PR body following one of our issue templates (https://github.com/paperclipai/paperclip/tree/master/.github/ISSUE_TEMPLATE). See CONTRIBUTING.md → "Link Issues or Describe Them In-PR".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: ea6e611

Important Issues (2)

• [gstack/review] server/src/services/activity.ts:343 — An empty action still fails open to the unfiltered audit feed. ?action= reaches the new truthiness check as "", so no SQL predicate is added, while server/src/routes/activity.ts:112 reports "action":"" in X-Applied-Filters. This recreates the exact false-verification failure the PR is intended to eliminate.
  Recommendation: parse the query with a strict schema that requires a non-empty action, or add the predicate whenever action !== undefined; add a route test for ?action=.
• [native-codex] server/src/routes/activity.ts:106 — Reflecting raw filter values into X-Applied-Filters makes valid Unicode filters fail with HTTP 500. For example, ?action=%E2%98%83 reaches JSON.stringify with the snowman intact, and Node rejects that header value with ERR_INVALID_CHAR. The activity write schema permits any non-empty string, so this is a valid stored action/query value rather than malformed input.
  Recommendation: avoid reflecting arbitrary query text in a response header, or encode the value into an ASCII-safe representation; add a Unicode action-filter route test.

Strengths

• The Drizzle eq predicate remains parameterized and company-scoped.
• Unknown parameter names now fail explicitly instead of silently widening the audit result.
• Route and embedded-Postgres tests cover the ordinary action-filter path and zero-match behavior.

Recommended Action

1. Address both Important issues before merge.
2. Re-run the focused route and service tests with empty and Unicode action cases.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head must be reopened under an independent author before an App approval is possible.

[kkroo]

Approved: action filtering is wired through service and route layers, unknown audit-route query keys now fail explicitly, response shape remains compatible, and route/service/OpenAPI tests cover the new behavior.

[allyblockcast]

Addressed both Important findings from Ally's review (head ea6e611, new head 5079654):

1. Empty ?action= failing open — services/activity.ts: filter checks now use !== undefined instead of truthiness, so an explicit empty-string filter narrows to zero rows instead of silently skipping the predicate. routes/activity.ts: the route now rejects an empty value for any of agentId/entityType/entityId/action with a 400 before it ever reaches the service, so ?action= can no longer masquerade as "filter applied" while returning the unfiltered feed.
2. Unicode crash in X-Applied-Filters — the header value is now encodeURIComponent-wrapped before res.setHeader, so a non-Latin1 action value (e.g. ?action=%E2%98%83) round-trips instead of throwing ERR_INVALID_CHAR.

Added route tests for the empty-action-400 case and the Unicode-action case, plus a service-level test asserting list({action: ""}) returns []. Updated the existing header-assertion tests to decodeURIComponent first.

Verified: activity-routes.test.ts + activity-service.test.ts + openapi-routes.test.ts — 23/23 passing. tsc --noEmit on server/ clean.

Note: this PR had reached the merge queue (position 39) after @kkroo's approval, which predates the review comment surfacing above by ~2.5 minutes — I dequeued it to land this fix rather than merge known-bad behavior, since the review findings were real correctness bugs on an audit-integrity surface. Re-requesting review on the new head.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 5079654

Prior Findings Dispositioned (2)

• prior:ea6e611 important 1 — fixed — server/src/routes/activity.ts:29 — action now requires at least one character, and the route returns 400 on schema failure before calling the service; the service also applies every explicitly supplied action predicate.
• prior:ea6e611 important 2 — fixed — server/src/routes/activity.ts:128 — the applied-filter JSON is now passed through encodeURIComponent before setHeader, and the Unicode route test exercises the corrected path.

Important Issues (1)

• [types/errors] server/src/routes/activity.ts:26 — The new runtime schema accepts any non-empty agentId, while the same endpoint documents agentId as a UUID in server/src/routes/openapi.ts:3056. A request such as ?agentId=not-a-uuid therefore reaches the Drizzle equality against the UUID column and PostgreSQL rejects it as invalid UUID syntax, turning malformed client input into a 500 instead of the documented 400.
  Recommendation: use z.string().uuid() in the runtime query schema and add a route or integration test proving an invalid UUID returns 400 without calling svc.list.

Strengths

• Empty action values now fail closed at both route and service boundaries.
• Applied-filter headers are ASCII-safe for Unicode filter values.
• The action predicate remains parameterized and company-scoped, with ordinary, zero-match, empty, and Unicode cases covered.

Recommended Action

1. Align runtime agentId validation with the documented UUID contract before merge.
2. Re-run the focused activity route and service tests.

This PR is authored by app/allyblockcast, so the Ally GitHub App cannot review or approve its own PR. The exact head 50796548832929f93bf49e49b333040a3769bfbe must be reopened under an independent author before an App approval is possible.