feat(web): add GET /api/connections/{id} detail endpoint - #1519
Open
Harsh23Kashyap wants to merge 15 commits into
Open
feat(web): add GET /api/connections/{id} detail endpoint#1519Harsh23Kashyap wants to merge 15 commits into
Harsh23Kashyap wants to merge 15 commits into
Conversation
Self-hosted operators have no way to monitor code-host connection sync
state from outside the settings UI. The page reads from Prisma via
`getConnectionsWithLatestJob` (a Server Action), which is not
reachable as `curl`. Operators who want to alert on a FAILED sync
have to scrape the database directly, bypassing the application
layer.
Add `GET /api/connections`:
- Auth-gated (any signed-in user, same as the settings page).
- Paginated via `page` and `perPage` (max 100, default 50).
- Per-connection response: id, name, connectionType, isDeclarative,
syncedAt, createdAt, updatedAt, repoCount, inFlightJobCount, latestJob
({id, status, createdAt, completedAt, errorMessage} or null).
- `latestJob` is the most recent ConnectionSyncJob for the
connection, or null if the connection has never been synced.
- `inFlightJobCount` is the count of PENDING + IN_PROGRESS jobs for
the connection, computed in a single grouped query rather than N+1
per-row counts.
- The connection `config` field (which carries tokens) is never in
the response.
- Response is sorted by name ascending, with `X-Total-Count` and
`Link` headers in the existing style.
The action lives in `listConnectionsApi.ts` and uses `sew` for
error wrapping and `withAuth` for auth, mirroring the
`/api/ee/audit` pattern.
Closes sourcebot-dev#1516.
Eleven vitest cases: - 401 when no authenticated user (`withAuth` returns a service error; the route forwards it as 401). - 200 + the documented shape on a happy path (id, name, connectionType, isDeclarative, syncedAt, createdAt, updatedAt, repoCount, inFlightJobCount, latestJob). - 200 with `latestJob: null` and `syncedAt: null` for a connection that has never been synced. - 200 with `inFlightJobCount` populated from the groupBy query. - 200 with the latest job's `errorMessage` exposed verbatim (the worker already scrubs the public `/api/health/ready` error strings, but this is an OWNER/admin endpoint; the worker message is exactly what an operator needs to debug a failing sync). - 200 with no `config` field in the response. - 400 for `perPage > 100`, `perPage <= 0`, and non-integer `page`. - 200 with empty list when no connections exist. - `X-Total-Count` and `Link` headers present on the response (with `rel="first"`, `rel="last"`, `rel="next"`). The mocks follow the project's established pattern: `vi.mock` `server-only`, `@sourcebot/db`, `@sourcebot/shared`, `@/lib/posthog`, `@/middleware/withAuth`. Added `@sentry/nextjs` and `@opentelemetry/sdk-trace-base` mocks to suppress a pre-existing version-mismatch failure (`sdk-trace-base` 1.28.0 expects `core.getEnv()` but the installed `@opentelemetry/core` 2.8.0 dropped it). The OpenTelemetry mock only needs to expose the symbol the `sew` import chain reaches at module-load time; it is a stub, not a real implementation.
The new endpoint is part of the public API surface (any signed-in
org member is the right access level — same as the settings UI),
so it needs to be in the public OpenAPI doc.
Add four schemas to `publicApiSchemas.ts`:
- `PublicListConnectionsQuery`: `page` and `perPage`, same shape
as the internal Zod schema.
- `PublicConnectionLatestJob`: id, status, createdAt, completedAt,
errorMessage.
- `PublicConnectionSummary`: id, name, connectionType, isDeclarative,
syncedAt, createdAt, updatedAt, repoCount, inFlightJobCount,
latestJob (nullable).
- `PublicListConnectionsResponse`: { connections: ConnectionSummary[] }.
Register the path in `publicApiDocument.ts` under the existing
`systemTag` (alongside `/api/version` and `/api/health`) with
the standard 200/400/401/500 response set and the
`X-Total-Count` + `Link` header descriptions reused from
`/api/repos`. The doc's auto-generated description makes the
auth requirement and the 'config is never returned' guarantee
explicit, since the latter is the only subtle part of the contract.
`yarn workspace @sourcebot/web openapi:generate` was rerun; the
regenerated `docs/api-reference/sourcebot-public.openapi.json`
includes the new `/api/connections` path and the four new schemas
in `components.schemas`.
`yarn workspace @sourcebot/web openapi:generate` output. The spec now lists the `/api/connections` path under the System tag and the four new components (PublicListConnectionsQuery, PublicConnectionLatestJob, PublicConnectionSummary, PublicListConnectionsResponse) under components.schemas. This commit is split out from the previous one so the schema + path registration is reviewable on its own, and the auto-generated spec is its own commit for the same reason.
The new endpoint shows up in the public OpenAPI spec; the docs.json "API Reference" -> "System" group needs the matching entry so Mintlify renders the page in the nav. Per the project changelog convention the entry is a single sentence with the issue link in the suffix. `sourcebot-dev#1516` is the issue this PR closes; once the PR is filed the suffix will be updated to `#XXXX (PR)`. Also adds a one-line changelog entry under [Unreleased] -> Added describing the new endpoint and pointing at the issue.
Address CodeRabbit finding on PR sourcebot-dev#1517: the project coding guidelines say "Always use curly braces for if statements, with the body on a new line, even for single-line bodies." The `if (linkHeader) headers.set('Link', linkHeader);` was a one-liner; wrap it in braces and put the body on its own line.
… is meaningful Address CodeRabbit security finding on PR sourcebot-dev#1517: the prior `buildPrismaMock` did not include a `config` field on the mocked Prisma rows, so the assertion `body.connections[0].config` is `undefined` was vacuously true. A future change that spreads the raw Prisma row into the response (e.g. `...connection` instead of the explicit field-by-field map) would leak the connection's credentials, and the test would still pass. Add a populated `config: { token: { env: 'SOURCEBOT_TEST_TOKEN' } }` to the mock. The test now actually exercises the strip-on-output path: if `listConnectionsApi.ts` ever returns the raw `config`, the assertion fails and the regression is caught.
…ssue sourcebot-dev#1516 Address CodeRabbit finding on PR sourcebot-dev#1517: per the project changelog convention, entries should link to the PR, not the issue.
…-dev#1508) * chore: upgrade seroval to 1.5.6 * docs: add changelog entry for seroval upgrade --------- Co-authored-by: Jack Minnetian <270441393+BlueBottleLatte@users.noreply.github.com> Co-authored-by: Brendan Kellam <brendan@sourcebot.dev>
Returns one connection in the org by id with its latest sync job, the count of in-flight jobs, and the most recent jobs (default 10, max 50 via ?jobLimit=). Auth-gated. The connection config (which carries tokens) is never returned. Cross-org access returns 404 (not 403) to avoid leaking the existence of connections in other orgs. Stacks on the GET /api/connections list endpoint (PR sourcebot-dev#1517) and reuses its connectionSummarySchema. Adds three new schemas to the public OpenAPI surface: PublicGetConnectionQuery, PublicConnectionJob, and PublicGetConnectionResponse. Refs sourcebot-dev#1518.
Adds the detail endpoint to the public OpenAPI spec and to the System API Reference navigation, and adds a one-sentence CHANGELOG entry under [Unreleased] -> Added linking to PR sourcebot-dev#1518. Refs sourcebot-dev#1518.
Extract the job-row-to-public-shape mapping into a single toConnectionJob helper. The latestJob and recentJobs responses now share the same projection, so a future change to the shape lives in one place. Also tightens the jobLimit test to assert that the action actually forwards the parsed jobLimit to Prisma's findMany take, instead of just checking the mock returned its own input. Refs sourcebot-dev#1518.
Contributor
WalkthroughAdds authenticated, organization-scoped endpoints to list connections and retrieve connection details with sync status, job history, validation, pagination, sensitive-field omission, tests, and public API documentation. ChangesConnections API
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route
participant Authenticated_Service
participant Prisma
Client->>API_Route: GET /api/connections or /api/connections/{id}
API_Route->>Authenticated_Service: Validate parameters and authorize request
Authenticated_Service->>Prisma: Query organization-scoped connections and sync jobs
Prisma-->>Authenticated_Service: Connection and job data
Authenticated_Service-->>API_Route: Public response payload
API_Route-->>Client: JSON response with status and headers
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9c510a8. Configure here.
Bugbot finding: the list endpoint fetched a per-connection count of ALL sync jobs but never read it. The endpoint exposes the latest job explicitly via the 'syncJobs: take 1' include, and the in-flight count via a separate groupBy — neither path needs the total count. Drop the unused aggregate so the list hot path doesn't run an unnecessary scan. Refs sourcebot-dev#1519.
Author
|
Good catch — fixed in a390fa83. The list endpoint's |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
CHANGELOG.md (1)
14-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMerge the duplicate
### Fixedsections.An existing
[Unreleased] → Fixedsection already appears below. Move the seroval entry there instead of creating a second### Fixedheading.🤖 Prompt for 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. In `@CHANGELOG.md` around lines 14 - 15, Move the seroval upgrade entry into the existing [Unreleased] → Fixed section in CHANGELOG.md, and remove the duplicate ### Fixed heading while preserving the entry text and link.Source: Coding guidelines
packages/web/src/app/api/(server)/connections/listConnectionsApi.ts (1)
24-35: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueDrop the unused
syncJobscount.Only
_count.reposis consumed (Line 67);_count.syncJobsadds a correlated COUNT per row for nothing.♻️ Proposed cleanup
_count: { select: { repos: true, - syncJobs: true, }, },🤖 Prompt for 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. In `@packages/web/src/app/api/`(server)/connections/listConnectionsApi.ts around lines 24 - 35, Remove the unused syncJobs field from the _count.select object in the listConnections query, while preserving _count.repos and the separately included latest syncJobs relation used by the response.packages/web/src/app/api/(server)/connections/[id]/route.test.ts (1)
414-423: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNeither suite asserts the org-scoping filter. Both Prisma mocks return fixtures regardless of arguments, so dropping
orgId: org.idfrom the queries would keep every test green — including the one labelled security-critical.
packages/web/src/app/api/(server)/connections/[id]/route.test.ts#L414-L423: assertconnection.findFirstwas called withwhere: { id: 42, orgId: 1 }.packages/web/src/app/api/(server)/connections/route.test.ts#L185-L230: assertconnection.findMany(andconnection.count) were called withwhere: { orgId: 1 }.🤖 Prompt for 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. In `@packages/web/src/app/api/`(server)/connections/[id]/route.test.ts around lines 414 - 423, Add assertions verifying the Prisma org-scoping filters: in packages/web/src/app/api/(server)/connections/[id]/route.test.ts lines 414-423, assert connection.findFirst receives where: { id: 42, orgId: 1 }; in packages/web/src/app/api/(server)/connections/route.test.ts lines 185-230, assert both connection.findMany and connection.count receive where: { orgId: 1 }. Use the existing Prisma mock spies without changing the response assertions.packages/web/src/app/api/(server)/connections/[id]/getConnectionApi.ts (1)
29-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument that
durationMsincludes queue wait.
createdAtis the job's enqueue timestamp, socompletedAt - createdAtis queue-wait + execution, not sync duration. Worth stating in the OpenAPI field description so operators don't misread it as sync time.🤖 Prompt for 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. In `@packages/web/src/app/api/`(server)/connections/[id]/getConnectionApi.ts around lines 29 - 31, Update the OpenAPI schema documentation for the durationMs field in the connection API to state that it measures total elapsed time from enqueue to completion, including queue wait and execution time, rather than sync execution time alone. Keep the existing calculation unchanged.packages/web/src/lib/schemas.ts (1)
50-67: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDerive
latestJobfromconnectionJobSchemato avoid duplicating the job shape.The inline
latestJobobject is a strict subset ofconnectionJobSchema(defined below at Line 77); keeping both hand-written means field/enum drift over time.♻️ Possible consolidation (requires moving `connectionJobSchema` above `connectionSummarySchema`)
- latestJob: z.object({ - id: z.string(), - status: z.nativeEnum(ConnectionSyncJobStatus), - createdAt: z.coerce.date(), - completedAt: z.coerce.date().nullable(), - errorMessage: z.string().nullable(), - }).nullable(), + latestJob: connectionJobSchema.omit({ + durationMs: true, + warningMessages: true, + }).nullable(),🤖 Prompt for 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. In `@packages/web/src/lib/schemas.ts` around lines 50 - 67, The inline latestJob schema in connectionSummarySchema duplicates connectionJobSchema and can drift. Move connectionJobSchema above connectionSummarySchema, then derive latestJob from it while preserving nullable behavior, removing the duplicated object definition.
🤖 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/web/src/app/api/`(server)/connections/route.ts:
- Line 1: Remove the 'use server' directive from the GET-only API route module
at packages/web/src/app/api/(server)/connections/route.ts#L1 and the
corresponding module at
packages/web/src/app/api/(server)/connections/[id]/route.ts#L1, leaving their
HTTP method exports unchanged.
In `@packages/web/src/lib/schemas.ts`:
- Around line 69-94: Update the list and get connection API route handlers to
validate their returned JSON with listConnectionsResponseSchema and
getConnectionResponseSchema, respectively. Keep the existing query-parameter
validation and response payloads intact, and ensure each handler parses the
response through the corresponding schema before returning it.
In `@packages/web/src/openapi/publicApiSchemas.ts`:
- Around line 120-125: Update publicGetConnectionResponseSchema to omit
latestJob from publicConnectionSummarySchema before extending it with the
nullable publicConnectionJobSchema, avoiding an allOf reference that preserves
the conflicting registered property. Follow the existing omit-then-extend
pattern in schemas.ts, and regenerate the OpenAPI specification afterward.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 14-15: Move the seroval upgrade entry into the existing
[Unreleased] → Fixed section in CHANGELOG.md, and remove the duplicate ### Fixed
heading while preserving the entry text and link.
In `@packages/web/src/app/api/`(server)/connections/[id]/getConnectionApi.ts:
- Around line 29-31: Update the OpenAPI schema documentation for the durationMs
field in the connection API to state that it measures total elapsed time from
enqueue to completion, including queue wait and execution time, rather than sync
execution time alone. Keep the existing calculation unchanged.
In `@packages/web/src/app/api/`(server)/connections/[id]/route.test.ts:
- Around line 414-423: Add assertions verifying the Prisma org-scoping filters:
in packages/web/src/app/api/(server)/connections/[id]/route.test.ts lines
414-423, assert connection.findFirst receives where: { id: 42, orgId: 1 }; in
packages/web/src/app/api/(server)/connections/route.test.ts lines 185-230,
assert both connection.findMany and connection.count receive where: { orgId: 1
}. Use the existing Prisma mock spies without changing the response assertions.
In `@packages/web/src/app/api/`(server)/connections/listConnectionsApi.ts:
- Around line 24-35: Remove the unused syncJobs field from the _count.select
object in the listConnections query, while preserving _count.repos and the
separately included latest syncJobs relation used by the response.
In `@packages/web/src/lib/schemas.ts`:
- Around line 50-67: The inline latestJob schema in connectionSummarySchema
duplicates connectionJobSchema and can drift. Move connectionJobSchema above
connectionSummarySchema, then derive latestJob from it while preserving nullable
behavior, removing the duplicated object definition.
🪄 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: 599456cf-c156-4f34-be3c-9d887e8596f7
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (12)
CHANGELOG.mddocs/api-reference/sourcebot-public.openapi.jsondocs/docs.jsonpackages/web/src/app/api/(server)/connections/[id]/getConnectionApi.tspackages/web/src/app/api/(server)/connections/[id]/route.test.tspackages/web/src/app/api/(server)/connections/[id]/route.tspackages/web/src/app/api/(server)/connections/listConnectionsApi.tspackages/web/src/app/api/(server)/connections/route.test.tspackages/web/src/app/api/(server)/connections/route.tspackages/web/src/lib/schemas.tspackages/web/src/openapi/publicApiDocument.tspackages/web/src/openapi/publicApiSchemas.ts
| @@ -0,0 +1,60 @@ | |||
| 'use server'; | |||
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -g 'route.ts' packages/web/src/app/api --exec sh -c 'head -3 "$1" | rg -q "use server" && echo "$1"' sh {}Repository: sourcebot-dev/sourcebot
Length of output: 1058
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== route.ts files with use server =="
fd -g 'route.ts' packages/web/src/app/api --print0 \
| xargs -0 -n1 sh -c 'if head -3 "$1" | rg -q "use server"; then echo "$1"; fi' sh
echo
echo "== top of targeted connection route handlers =="
for f in 'packages/web/src/app/api/(server)/connections/route.ts' 'packages/web/src/app/api/(server)/connections/[id]/route.ts'; do
echo "--- $f"
sed -n '1,120p' "$f" | nl -ba | sed -n '1,120p'
done
echo
echo "== deterministic module parsing: use-server declarations, default route exports, server action exports in targeted files =="
python3 - <<'PY'
from pathlib import Path
for p in [
Path('packages/web/src/app/api/(server)/connections/route.ts'),
Path('packages/web/src/app/api/(server)/connections/[id]/route.ts'),
]:
text = p.read_text()
lines = text.splitlines()
print(f'--- {p}')
for i, line in enumerate(lines[:120], 1):
s=line.strip()
if s.startswith('use ') or s.startswith("'use") or s.startswith('export ') or s.startswith('function '):
print(f'{i}: {line}')
PY
echo
echo "== package framework / eslint / server action config hints =="
for f in packages/web/package.json next.config.* .eslintrc* eslint.config.*; do
[ -e $f ] && echo "--- $f" && sed -n '1,160p' "$f"
doneRepository: sourcebot-dev/sourcebot
Length of output: 1244
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== targeted route file headers =="
for f in 'packages/web/src/app/api/(server)/connections/route.ts' 'packages/web/src/app/api/(server)/connections/[id]/route.ts'; do
echo "--- $f"
sed -n '1,140p' "$f" | awk '{printf "%5d\t%s\n", NR, $0}'
done
echo
echo "== all server/api route declarations that start with use server =="
python3 - <<'PY'
from pathlib import Path
for p in Path('packages/web/src/app/api').rglob('route.ts'):
text = p.read_text(errors='replace')
first = text.splitlines()[0].strip() if text.splitlines() else ''
if first in {"'use server'", "\"use server\""}:
print(p)
PY
echo
echo "== package/framework/config hints =="
for f in packages/web/package.json next.config.* next.config.ts next.config.mjs .eslintrc .eslintrc.js .eslintrc.json eslint.config.js eslint.config.mjs eslint.config.cjs; do
if [ -e "$f" ]; then
echo "--- $f"
sed -n '1,180p' "$f"
fi
doneRepository: sourcebot-dev/sourcebot
Length of output: 11368
Remove 'use server' from these API route handlers. These modules only export HTTP methods (GET), so the directive turns the route handler exports into Server Actions, which is a different semantic from an API route.
packages/web/src/app/api/(server)/connections/route.ts#L1packages/web/src/app/api/(server)/connections/[id]/route.ts#L1
📍 Affects 2 files
packages/web/src/app/api/(server)/connections/route.ts#L1-L1(this comment)packages/web/src/app/api/(server)/connections/[id]/route.ts#L1-L1
🤖 Prompt for 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.
In `@packages/web/src/app/api/`(server)/connections/route.ts at line 1, Remove the
'use server' directive from the GET-only API route module at
packages/web/src/app/api/(server)/connections/route.ts#L1 and the corresponding
module at packages/web/src/app/api/(server)/connections/[id]/route.ts#L1,
leaving their HTTP method exports unchanged.
Comment on lines
+69
to
+94
| export const listConnectionsResponseSchema = z.object({ | ||
| connections: z.array(connectionSummarySchema), | ||
| }); | ||
|
|
||
| export const getConnectionQueryParamsSchema = z.object({ | ||
| jobLimit: z.coerce.number().int().positive().max(50).default(10), | ||
| }); | ||
|
|
||
| export const connectionJobSchema = z.object({ | ||
| id: z.string(), | ||
| status: z.nativeEnum(ConnectionSyncJobStatus), | ||
| createdAt: z.coerce.date(), | ||
| completedAt: z.coerce.date().nullable(), | ||
| durationMs: z.number().int().nonnegative().nullable(), | ||
| errorMessage: z.string().nullable(), | ||
| warningMessages: z.array(z.string()), | ||
| }); | ||
|
|
||
| export const getConnectionResponseSchema = z.object({ | ||
| connection: connectionSummarySchema.omit({ | ||
| latestJob: true, | ||
| }).extend({ | ||
| latestJob: connectionJobSchema.nullable(), | ||
| }), | ||
| recentJobs: z.array(connectionJobSchema), | ||
| }); No newline at end of file |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts '\b(listConnectionsResponseSchema|getConnectionResponseSchema|connectionSummarySchema|connectionJobSchema)\b'Repository: sourcebot-dev/sourcebot
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked files containing schemas.ts:"
fd -a 'schemas\.ts$' . || true
echo
echo "All exact references across repo:"
rg -n '\b(listConnectionsResponseSchema|getConnectionResponseSchema|connectionSummarySchema|connectionJobSchema)\b' . || true
echo
echo "Candidate schema file snippets:"
for f in $(fd 'schemas\.ts$' .); do
echo "--- $f"
wc -l "$f"
rg -n "export const (listConnectionsResponseSchema|getConnectionResponseSchema|connectionSummarySchema|connectionJobSchema|getConnectionQueryParamsSchema|listConnectionsQueryParamsSchema)" "$f" -A 8 -B 3 || true
doneRepository: sourcebot-dev/sourcebot
Length of output: 3166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Exports/import usages of connection schemas:"
rg -n '\b(connectionSummarySchema|connectionJobSchema|listConnectionsResponseSchema|getConnectionResponseSchema)\b' .
echo
echo "Exports/import usages of query-param schemas:"
rg -n '\b(listConnectionsQueryParamsSchema|getConnectionQueryParamsSchema)\b' .
echo
echo "Relevant route/action references around list/get connection:"
rg -n '\b(listConnections|getConnection|connections)' packages/web/src -g '*.{ts,tsx}' | head -200Repository: sourcebot-dev/sourcebot
Length of output: 13290
Add response validation for the new connection API endpoints.
listConnectionsResponseSchema and getConnectionResponseSchema are only defined in packages/web/src/lib/schemas.ts; the list/get route handlers import the query-param schemas but never parse the returned JSON. Parse the route responses with these schemas too so response shape drift is caught.
🤖 Prompt for 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.
In `@packages/web/src/lib/schemas.ts` around lines 69 - 94, Update the list and
get connection API route handlers to validate their returned JSON with
listConnectionsResponseSchema and getConnectionResponseSchema, respectively.
Keep the existing query-parameter validation and response payloads intact, and
ensure each handler parses the response through the corresponding schema before
returning it.
- CHANGELOG: move the seroval upgrade entry into the existing [Unreleased] -> Fixed section (drops a duplicate heading introduced by the rebase). - schemas.ts: move connectionJobSchema above connectionSummarySchema and derive `connectionSummarySchema.latestJob` from it via .omit() so the two job shapes can't drift apart. - publicApiSchemas.ts: apply the same omit-then-extend pattern to publicGetConnectionResponseSchema, and document durationMs as elapsed-from-enqueue time (includes queue wait) and jobLimit in the OpenAPI spec. - route.test.ts: add explicit Prisma-call assertions on the org-scoping filter for both the happy path and the 404 path. The mock returns its fixture regardless of args, so without these assertions a future change that drops `orgId: org.id` from the where clause would still pass the 404 test. All 35 web tests pass; OpenAPI spec regenerated. Refs sourcebot-dev#1519.
Author
|
All 5 nitpicks + Bugbot finding addressed in b51e4203 and a390fa83:
Skipped: the inline suggestion to remove OpenAPI spec regenerated; 35/35 web tests pass. |
Add Prisma-call assertions on the where: { orgId: org.id } filter for
both connection.findMany and connection.count. The mock returns its
fixture regardless of args, so without these a future change that
drops the org filter from the where clause would still pass the
response-shape assertions.
Refs sourcebot-dev#1519.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
GET /api/connections/{id}endpoint that returns one connection in the org by id, plus its most recent sync jobs and the count of in-flight jobs. Stacks on PR #1517 (the list endpoint) and reuses itsconnectionSummarySchemato keep the row shape consistent across both endpoints.Fixes #1518.
What it returns
{ "connection": { "id": 1, "name": "github-public", "connectionType": "github", "isDeclarative": false, "syncedAt": "2026-07-25T13:59:30.000Z", "createdAt": "2026-06-12T08:21:00.000Z", "updatedAt": "2026-07-25T13:59:30.000Z", "repoCount": 12, "inFlightJobCount": 0, "latestJob": { "id": "ckxxx...", "status": "COMPLETED", "createdAt": "2026-07-25T13:59:00.000Z", "completedAt": "2026-07-25T13:59:30.000Z", "durationMs": 30000, "errorMessage": null, "warningMessages": [] } }, "recentJobs": [ /* up to ?jobLimit= (default 10, max 50) */ ] }Design decisions
?jobLimit=defaults to 10, capped at 50. Atakeof 50 is plenty for an operator inspecting why a connection is broken; the cap prevents an unboundedfindManyfrom a misbehaving client. The recent-jobs list and the embeddedlatestJobalways agree (both come from the samefindMany).connectionSyncJob.groupBy, not N+1 per-connection counts. Matches the list endpoint's pattern in PR feat(web): add GET /api/connections admin endpoint for monitoring connection sync state #1517.where: { id, orgId: org.id }collapses that case to "not found in this org" — same pattern the list endpoint uses.configis never in the response. It carries tokens. The regression test for this is a deliberately populatedconfigin the Prisma mock so a future change that spreads raw Prisma rows fails the test.errorMessageandwarningMessagesare exposed verbatim. The endpoint is auth-gated (any user in the org, not just owners), and the worker message is exactly what an operator needs to debug a failing sync. The warning list is also exposed for the "8 repos were skipped" case that comes up on GitHub Apps with limited permissions.durationMsis computed server-side fromcompletedAt - createdAt, so clients don't have to.nullwhile the job is still running.connectionSummarySchemaand the consistentlatestJobrow shape (withoutdurationMsorwarningMessages, since the list endpoint doesn't return full job rows). When feat(web): add GET /api/connections admin endpoint for monitoring connection sync state #1517 merges, this PR re-resolves onmaincleanly.Test coverage
13 vitest cases in
route.test.ts:idis not a positive integeridis zero or negativeconfigfield never leaks to the responseerrorMessageandwarningMessagesare passed through verbatimjobLimitis forwarded to Prisma'sfindManyastakejobLimit=0jobLimit > 50recentJobsfor an un-synced connectioninFlightJobCountpopulated from thegroupByqueryAll 35 web tests pass (13 new + 22 pre-existing). No new TypeScript errors.
OpenAPI / docs
systemTaginpublicApiDocument.ts.docs/api-reference/sourcebot-public.openapi.jsonviayarn openapi:generate.docs/docs.jsonunder the System group.[Unreleased] → Addedlinking to this PR.Risks
withAuth(any user in the org) as the list endpoint. Connection names, types, and recent sync state are exposed to any user who can sign in.Future work
connection_sync_job.statuschanges (a polling model is fine for a few connections; not for hundreds).Note
Medium Risk
New org-scoped read APIs expose connection names and sync job errors to any authenticated org member; correctness of org filtering and omitting
configis security-sensitive, though heavily tested.Overview
Introduces authenticated public APIs so operators can inspect code-host connection health without using the UI.
GET /api/connectionsreturns a paginated org-scoped list with sync metadata (syncedAt,repoCount,inFlightJobCount, trimmedlatestJob), usingX-Total-Countand RFC 8288Linkheaders like other list routes.GET /api/connections/{id}returns one connection with a fullerlatestJob(including server-computeddurationMsandwarningMessages) andrecentJobscapped by?jobLimit=(default 10, max 50). Both handlers map Prisma rows explicitly soconfig(tokens) is never serialized; cross-org lookups useorgIdin the query and respond with 404.Shared Zod schemas in
schemas.tsand OpenAPI registration/schemas are updated, with docs navigation and changelog entries.serovalis bumped to ^1.5.6 in the lockfile. Broad route tests cover auth, validation, pagination headers, and the security regressions above.Reviewed by Cursor Bugbot for commit 603e0ac. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
serovalto^1.5.6.