feat(api): headless agent turn endpoint (POST /api/v1/projects/:projectId/agent) - #3629
Conversation
…rn for the Slack app One assistant turn over a caller-supplied message history, run through the shared engine facade (runUnifiedAssistantTurn) on the hosted /stream rail with a pinned hosted model, billed to the path project. - Tools: platform-op reads + atomic create_eval_suite only. Run/cancel and generate_eval_cases (spend ops) are excluded — runs stay human-gated caller-side via POST /eval-runs. Every op input is hard-clamped to the route projectId; the in-app WORKSPACE_OPERATIONS gate is untouched. - Auth: the delegated org-scoped Convex JWT backs both the engine's /stream calls and the self-dispatched platform-op calls (keeps the agent's own tool calls out of the caller's per-key rate bucket). - Docs MCP server mounted with preflight-degrade; skillsSource none; no tasks seam; no chat-session persistence (caller owns transcript). - Caps: 4 concurrent turns/org (429), 12 steps, 90s wall clock (504); strict input limits (50 msgs × 8KB). Engine cap/quota failures map to RATE_LIMITED; missing turnTrace maps to INTERNAL_ERROR. - createdResources collected via an execution wrapper BEFORE the 24KB model-facing result cap, with app deep links. - Server-authoritative telemetry event api_agent_turn_completed (names/counts/durations only). - OpenAPI + public-api.mdx entries; 13 route/tool-adapter tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
GitHub Actions
Inactive
dosubot
Bot
added
size:XL
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_0f689021-b97c-47c6-8387-dfaf9406d3ba) |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its
|
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
WalkthroughThe change adds an authenticated 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
mcpjam-inspector/server/routes/v1/__tests__/agent.test.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
mcpjam-inspector/server/routes/v1/agent.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: ESLint configuration in --config is invalid:
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts (2)
292-353: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestore the operation spies unconditionally.
Both tests call
vi.spyOnonlistProjectServersOperationandcreateEvalSuiteOperation, which are shared singletons imported from@mcpjam/sdk/platform.mockRestore()runs on the last line of each test, so a failing assertion above it leaves the spy attached for every subsequent test in the worker. TheafterEach(vi.clearAllMocks)at Line 163 is scoped to the firstdescribeand does not cover this block. AnafterEachwithvi.restoreAllMocks()makes the teardown unconditional.Based on learnings, the
30_000fixture is scenario data for the truncation path, not a canonical cap, so I make no claim aboutMODEL_OUTPUT_CAPfrom it.🧪 Proposed fix
describe("agent tool surface", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + it("keeps spend ops out of the op list and the in-app gate unchanged", () => {🤖 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 `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts` around lines 292 - 353, Add an afterEach teardown for these tests that calls vi.restoreAllMocks(), ensuring spies on listProjectServersOperation.execute and createEvalSuiteOperation.execute are restored even when assertions fail. Remove the per-test mockRestore calls if the shared teardown makes them redundant.Source: Learnings
251-278: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
504 TIMEOUTpath.The suite exercises every documented failure mapping except the wall-clock one.
agent.tsLines 439-450 returnTIMEOUTwhenabortController.signal.abortedis true after the turn resolves, and bothdocs/reference/openapi.json(Line 3446) and the public guide publish that504. A mock that aborts the suppliedabortSignalbefore resolving would pin the branch down without fake timers.🧪 Sketch
+ it("maps a wall-clock abort to TIMEOUT", async () => { + runUnifiedAssistantTurnMock.mockImplementation(async (opts: any) => { + (opts.abortSignal as AbortSignal).dispatchEvent?.(new Event("abort")); + // Simpler: have the route's controller abort by stubbing the timer, + // or expose the signal and abort it here before resolving. + return okTurnResult({ aborted: true }); + }); + const res = await turnRequest(makeApp(), OK_BODY); + expect(res.status).toBe(504); + const body = (await res.json()) as { code: string }; + expect(body.code).toBe("TIMEOUT"); + });🤖 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 `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts` around lines 251 - 278, Add a test covering the timeout mapping in the agent request suite by making runUnifiedAssistantTurnMock abort the supplied abortSignal before resolving successfully, then assert turnRequest returns HTTP 504 with the TIMEOUT response. Keep the mock scoped to this test and preserve existing concurrency and failure-mapping tests.mcpjam-inspector/server/routes/v1/agent.ts (2)
230-243: 🩺 Stability & Availability | 🔵 TrivialThe concurrency cap is per process, not per organization.
activeTurnsByOrgis module state. If the inspector runs more than one instance behind a load balancer, the effective ceiling becomes 4 × instances, whiledocs/reference/openapi.json(Line 3376) anddocs/reference/public-api.mdx(Line 282) both promise a flat 4 per organization. The code comment acknowledges the in-process scope; the public contract does not. Either soften the documented wording to "approximately" or move the counter to shared state when the deployment scales horizontally.🤖 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 `@mcpjam-inspector/server/routes/v1/agent.ts` around lines 230 - 243, Align the concurrency limit with the documented per-organization contract by replacing module-local activeTurnsByOrg tracking with shared state that coordinates acquireTurnSlot and releaseTurnSlot across all inspector instances; otherwise update the referenced OpenAPI and public API documentation to explicitly describe the limit as approximate and process-local.
211-228: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider a total-history character budget alongside the per-message cap.
MAX_MESSAGESandMAX_MESSAGE_CHARSbound each message, but nothing bounds their product. A caller may send 50 messages of 8,000 characters each, so one request can carry roughly 400,000 characters of prompt into a hosted turn billed to the project. The concurrency cap limits parallel turns, not the size of any single one. A summed budget check after parsing would close that gap cheaply.♻️ Sketch
const agentTurnSchema = z.object({ messages: z .array( z.object({ role: z.enum(["user", "assistant"]), content: z.string().min(1).max(MAX_MESSAGE_CHARS), }) ) .min(1) - .max(MAX_MESSAGES), + .max(MAX_MESSAGES) + .refine( + (messages) => + messages.reduce((sum, m) => sum + m.content.length, 0) <= + MAX_TOTAL_HISTORY_CHARS, + { error: "Conversation history is too large for one turn." } + ), });🤖 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 `@mcpjam-inspector/server/routes/v1/agent.ts` around lines 211 - 228, Update agentTurnSchema validation to enforce a total character budget across all message content in addition to the existing per-message and message-count limits. Add a post-parse/refinement check over the messages array, using a named aggregate-history limit, and reject requests whose summed content exceeds that limit before the hosted turn is started.
🤖 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 `@docs/reference/openapi.json`:
- Around line 3400-3449: Add a 500 response entry to the operation’s responses
map, referencing the existing internal-error response component used for
INTERNAL_ERROR outcomes. Keep the current 400, 401, 422, 429, and 504 response
definitions unchanged.
In `@mcpjam-inspector/server/routes/v1/agent.ts`:
- Around line 349-373: Update the docs preflight around manager.listTools in the
wall-clock-controlled turn flow so it observes the existing abortController
signal and cannot outlive TURN_WALL_CLOCK_MS. Race the preflight against that
signal or allocate only the remaining turn budget, preserving the current
selected-server and warning behavior while ensuring an expired budget propagates
to the existing 504 handling.
- Around line 494-500: Harden the cleanup in the finally block around
manager.disconnectAllServers(): normalize both synchronous throws and promise
rejections so cleanup cannot override the successful response, and log any
disconnect failure at debug level instead of silently swallowing it. Preserve
the existing timeout clearing and turn-slot release behavior.
---
Nitpick comments:
In `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts`:
- Around line 292-353: Add an afterEach teardown for these tests that calls
vi.restoreAllMocks(), ensuring spies on listProjectServersOperation.execute and
createEvalSuiteOperation.execute are restored even when assertions fail. Remove
the per-test mockRestore calls if the shared teardown makes them redundant.
- Around line 251-278: Add a test covering the timeout mapping in the agent
request suite by making runUnifiedAssistantTurnMock abort the supplied
abortSignal before resolving successfully, then assert turnRequest returns HTTP
504 with the TIMEOUT response. Keep the mock scoped to this test and preserve
existing concurrency and failure-mapping tests.
In `@mcpjam-inspector/server/routes/v1/agent.ts`:
- Around line 230-243: Align the concurrency limit with the documented
per-organization contract by replacing module-local activeTurnsByOrg tracking
with shared state that coordinates acquireTurnSlot and releaseTurnSlot across
all inspector instances; otherwise update the referenced OpenAPI and public API
documentation to explicitly describe the limit as approximate and process-local.
- Around line 211-228: Update agentTurnSchema validation to enforce a total
character budget across all message content in addition to the existing
per-message and message-count limits. Add a post-parse/refinement check over the
messages array, using a named aggregate-history limit, and reject requests whose
summed content exceeds that limit before the hosted turn is started.
🪄 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: 3f4d474a-852f-40c1-8695-d258c0a980bb
📒 Files selected for processing (7)
docs/reference/openapi.jsondocs/reference/public-api.mdxmcpjam-inspector/server/routes/v1/__tests__/agent.test.tsmcpjam-inspector/server/routes/v1/agent.tsmcpjam-inspector/server/routes/v1/index.tsmcpjam-inspector/server/utils/built-in-tools/mcpjam.tsmcpjam-inspector/shared/analytics-events.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 644a366a6c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return v1Error( | ||
| c, | ||
| "TIMEOUT", | ||
| `Agent turn exceeded the ${TURN_WALL_CLOCK_MS / 1000}s limit.` | ||
| ); |
There was a problem hiding this comment.
If create_eval_suite completes and the subsequent model step times out, created already contains the persisted suite, but this error response discards it; the missing-turnTrace branch below does the same after a post-tool engine failure. A caller retrying the apparently failed request can therefore create duplicate suites without ever receiving the first suite's ID. Return the collected resource references in these error responses or otherwise make the mutation idempotent.
Useful? React with 👍 / 👎.
| for (const operation of AGENT_API_OPERATIONS) { | ||
| tools[operation.name] = tool({ | ||
| description: `${operation.description} (Scoped to the current project automatically.)`, | ||
| inputSchema: operation.inputSchema, |
There was a problem hiding this comment.
getEvalRunOperation is the one included operation whose input schema requires project, so reusing its schema tells the model that this field is mandatory while the system prompt tells it to omit the field and never exposes the route's project ID. For a user who supplies only a run ID, the model must either violate the advertised schema or invent a selector that this wrapper rejects, making the promised run lookup unreliable unless an earlier tool happened to reveal the exact project ID. Adapt this schema so project is optional or absent, then inject the path project as the wrapper already does.
Useful? React with 👍 / 👎.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mcpjam-inspector/server/routes/v1/agent.ts">
<violation number="1" location="mcpjam-inspector/server/routes/v1/agent.ts:306">
P1: Concurrent JWT-authenticated turns across every organization share the four-slot `anonymous` bucket, so one tenant can exhaust this endpoint globally. Derive an authenticated organization key for JWT callers or use a project-scoped fallback instead of a shared anonymous key.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ); | ||
|
|
||
| const orgKey = | ||
| c.get("mcpjamOrganizationId") ?? c.get("workosUserId") ?? "anonymous"; |
There was a problem hiding this comment.
P1: Concurrent JWT-authenticated turns across every organization share the four-slot anonymous bucket, so one tenant can exhaust this endpoint globally. Derive an authenticated organization key for JWT callers or use a project-scoped fallback instead of a shared anonymous key.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/server/routes/v1/agent.ts, line 306:
<comment>Concurrent JWT-authenticated turns across every organization share the four-slot `anonymous` bucket, so one tenant can exhaust this endpoint globally. Derive an authenticated organization key for JWT callers or use a project-scoped fallback instead of a shared anonymous key.</comment>
<file context>
@@ -0,0 +1,538 @@
+ );
+
+ const orgKey =
+ c.get("mcpjamOrganizationId") ?? c.get("workosUserId") ?? "anonymous";
+ if (!acquireTurnSlot(orgKey)) {
+ return v1Error(
</file context>
- Docs preflight bounded: raced against a 5s deadline + the turn's abort signal, so a slow docs server can no longer stack its 30s connect timeout on top of the 90s turn budget (coderabbit major, cubic P2). - JWT callers no longer share one global 'anonymous' concurrency bucket — fall back to a project-scoped key (cubic P1). - Failed/timed-out turns surface already-persisted suites in the error body's details.createdResources, so retries don't double-create (codex P1, cubic P2); documented in the OpenAPI description. - Advertised tool schemas mark project optional even where the op requires it (get_eval_run), removing the schema-vs-prompt contradiction (codex/cubic P2). - otherProjects switching metadata stripped from op results — a clamped surface must not disclose the org's other projects (cubic P2). - Message quota enforced in BYTES (8192) as well as chars — the char-only cap was 4x bypassable with multibyte text (cubic P2). - finally-cleanup guarded against synchronous throws (coderabbit minor). - OpenAPI documents the 500 response (cubic/coderabbit P3). - Tests: +6 (docs-available path, hung-preflight degrade under fake timers, byte cap, created-on-failure details, optional-project schema, otherProjects strip) — 19 in agent.test.ts, full v1 suite 253 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_211f1ae5-54e1-4e4a-8c52-bfd5381b51b4) |
|
Pushed 24c2fa6 addressing all bot review findings:
agent.test.ts is up to 19 tests; full v1 + built-in-tools suites: 253 passed. openapi-drift green. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24c2fa6b72
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const deadline = setTimeout(() => { | ||
| logger.warn("[v1/agent] docs MCP preflight timed out; continuing"); | ||
| resolve(false); | ||
| }, DOCS_PREFLIGHT_TIMEOUT_MS); |
There was a problem hiding this comment.
When manager.listTools() succeeds or rejects before five seconds, Promise.race settles but this deadline remains armed, so it later emits a false “preflight timed out” warning for every normally completed preflight (and duplicates the warning on fast failures). Because server warnings are forwarded to Sentry, this creates misleading production alerts; retain the timer handle and clear it when either preflight branch wins.
AGENTS.md reference: mcpjam-inspector/AGENTS.md:L22-L22
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/reference/openapi.json (1)
6508-6617: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDocument the
contentbyte cap inAgentTurnRequest.
AgentTurnRequestonly advertisesmaxLength: 8000, butv1/agent.tsalso enforcesBuffer.byteLength(..., "utf8") <= 8192, so a schema-valid 8,000-character UTF-8 message can still return 400. Add the byte limit to the OpenAPI property contract so callers know both constraints.🤖 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 `@docs/reference/openapi.json` around lines 6508 - 6617, Update the AgentTurnRequest.messages item content schema to document the enforced UTF-8 byte limit alongside maxLength: 8000. Add the appropriate OpenAPI byte-cap constraint to the content property while preserving its existing character-length validation.
🧹 Nitpick comments (1)
mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts (1)
323-341: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCover project-scoped concurrency isolation.
The test at Line 323 sends every request to
p1. An organization-wide bucket would pass this test, even though it violates the required project-scoped behavior. Hold fourp1turns, then start ap2turn with the same delegated caller and assert that it reaches the engine.As per PR objectives, use project-scoped concurrency buckets for JWT callers.
🤖 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 `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts` around lines 323 - 341, Extend the “caps concurrent turns per organization” test to verify project-scoped isolation for JWT callers: keep four held turns targeting project p1, then issue a turn for project p2 using the same delegated caller and assert it reaches the engine successfully rather than receiving 429. Ensure the test releases all held gates and awaits the added request while preserving the existing p1 saturation assertion.
🤖 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 `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts`:
- Around line 397-416: The test mock for listProjectServersOperation.execute
uses the wrong result property. Update the mock in “strips otherProjects
switching metadata from results” to return items: [] per the operation contract,
and assert result.items equals [] while retaining the otherProjects omission
assertion.
---
Outside diff comments:
In `@docs/reference/openapi.json`:
- Around line 6508-6617: Update the AgentTurnRequest.messages item content
schema to document the enforced UTF-8 byte limit alongside maxLength: 8000. Add
the appropriate OpenAPI byte-cap constraint to the content property while
preserving its existing character-length validation.
---
Nitpick comments:
In `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts`:
- Around line 323-341: Extend the “caps concurrent turns per organization” test
to verify project-scoped isolation for JWT callers: keep four held turns
targeting project p1, then issue a turn for project p2 using the same delegated
caller and assert it reaches the engine successfully rather than receiving 429.
Ensure the test releases all held gates and awaits the added request while
preserving the existing p1 saturation assertion.
🪄 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: e789af58-20f9-4e15-858e-83dd99f0f4a9
📒 Files selected for processing (3)
docs/reference/openapi.jsonmcpjam-inspector/server/routes/v1/__tests__/agent.test.tsmcpjam-inspector/server/routes/v1/agent.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- mcpjam-inspector/server/routes/v1/agent.ts
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="mcpjam-inspector/server/routes/v1/agent.ts">
<violation number="1" location="mcpjam-inspector/server/routes/v1/agent.ts:365">
P1: Unauthenticated or JWT callers can bypass the four-turn gate by varying `projectId`; a member can also multiply capacity across projects in one org. Authenticate/resolve an organization before acquiring the slot, then use that organization as the key rather than an untrusted path parameter.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const orgKey = | ||
| c.get("mcpjamOrganizationId") ?? | ||
| c.get("workosUserId") ?? | ||
| `project:${projectId}`; |
There was a problem hiding this comment.
P1: Unauthenticated or JWT callers can bypass the four-turn gate by varying projectId; a member can also multiply capacity across projects in one org. Authenticate/resolve an organization before acquiring the slot, then use that organization as the key rather than an untrusted path parameter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At mcpjam-inspector/server/routes/v1/agent.ts, line 365:
<comment>Unauthenticated or JWT callers can bypass the four-turn gate by varying `projectId`; a member can also multiply capacity across projects in one org. Authenticate/resolve an organization before acquiring the slot, then use that organization as the key rather than an untrusted path parameter.</comment>
<file context>
@@ -302,8 +355,14 @@ agent.post("/projects/:projectId/agent", async (c) => {
- c.get("mcpjamOrganizationId") ?? c.get("workosUserId") ?? "anonymous";
+ c.get("mcpjamOrganizationId") ??
+ c.get("workosUserId") ??
+ `project:${projectId}`;
if (!acquireTurnSlot(orgKey)) {
return v1Error(
</file context>
…sconnect fixes - Retry contract made explicit: the operation is documented (route docblock, OpenAPI, public-api.mdx) as NOT idempotent with a no-blind-retry policy — dedupe on trigger identity, check details.createdResources before retrying. Durable Idempotency-Key support through the mutation path is called out as the prerequisite for going beyond dogfood. - Aggregate history budget: 96 KB total UTF-8 cap on the message array (per-message caps alone admitted ~400 KB per request). - Docs preflight timer disarmed when the race settles — a successful preflight no longer leaves a live 5s timer emitting a false timeout warning. - Caller disconnects propagate: c.req.raw.signal aborts the turn, so an abandoned request stops consuming model capacity. - Teardown failures are logged (observably detached, not silently swallowed). - Suite default model decided: authored suites default to anthropic/claude-haiku-4.5 (the cheap eval workhorse), not the agent's own Sonnet 5 — suites run every case × iteration on schedules. - OpenAPI: byte caps + process-local concurrency documented. - Test fixtures corrected to list_project_servers' real `items` field; +2 tests (aggregate budget, disconnect abort) — 21 in agent.test.ts, full v1 suite 255 green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Error
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_1cf3e607-26c2-429c-80b7-1bd812a31579) |
|
Pushed 9d1ff5d for the round-2 review:
Cleanup: teardown failures are logged instead of silently swallowed; test fixtures use 21 agent tests / 255 full v1 suite green, tsc clean. The preview redeploy for this head should follow from the push — the stale status comment was pointing at the pre-round-1 commit. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d1ff5dff0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!result.turnTrace) { | ||
| const message = | ||
| lastEngineError?.message ?? | ||
| "Agent turn failed: the engine returned no turn trace."; | ||
| const rateLimited = | ||
| lastEngineError?.httpStatus === 429 || | ||
| rt.classifyFailure(message) === "rate_limited"; |
There was a problem hiding this comment.
When the hosted /stream request returns a non-OK response such as 429 or 500, runChatEngineLoop invokes onEngineError but returns normally; its epilogue marks the run successful and builds a turnTrace (mcpjam-stream-handler.ts:2250-2260, 3273-3279, 3343-3345). Consequently this condition is false despite lastEngineError being populated, and the route responds 200—often with an empty reply—instead of the documented RATE_LIMITED or INTERNAL_ERROR. Check lastEngineError independently of the missing-trace fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts (3)
168-187: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd a delegated-authentication failure test.
The suite covers a missing bearer token and a valid guest token. It does not make
getConvexBearerMockreturn no delegated token or reject. A regression in delegated JWT validation could therefore pass all authentication tests. Add a case that expects401withUNAUTHORIZEDand verifiesrunUnifiedAssistantTurnMockis not called.🤖 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 `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts` around lines 168 - 187, Add a test in the agent authentication suite that configures getConvexBearerMock to return no delegated token or reject, then sends a request through the existing turnRequest/makeApp flow. Assert a 401 response with code UNAUTHORIZED and verify runUnifiedAssistantTurnMock was not called.
246-260: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAssert documentation preflight teardown.
This test verifies the fallback response but not cleanup.
managerDisconnectMockis configured but never asserted. A regression could return200while leaving the documentation manager active. Assert that teardown runs after the timeout.🤖 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 `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts` around lines 246 - 260, Extend the hanging documentation preflight test around turnRequest to assert that managerDisconnectMock is called after the 5-second timeout fallback completes. Keep the existing response and selectedServers assertions, and verify teardown occurs before restoring real timers.
383-392: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAssert the complete operation allowlist.
This test excludes only three known operations. It can remain green if another write or spend operation is added to
AGENT_API_OPERATIONS. Assert the complete approved set, or verify that every operation is read-only exceptcreateEvalSuiteOperation.🤖 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 `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts` around lines 383 - 392, Update the “agent tool surface” test to validate the complete AGENT_API_OPERATIONS allowlist rather than excluding only the currently known operations. Assert the exact approved operation names, or verify that every listed operation is read-only with createEvalSuiteOperation as the sole exception, while preserving the existing in-app gate assertion.
🤖 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.
Nitpick comments:
In `@mcpjam-inspector/server/routes/v1/__tests__/agent.test.ts`:
- Around line 168-187: Add a test in the agent authentication suite that
configures getConvexBearerMock to return no delegated token or reject, then
sends a request through the existing turnRequest/makeApp flow. Assert a 401
response with code UNAUTHORIZED and verify runUnifiedAssistantTurnMock was not
called.
- Around line 246-260: Extend the hanging documentation preflight test around
turnRequest to assert that managerDisconnectMock is called after the 5-second
timeout fallback completes. Keep the existing response and selectedServers
assertions, and verify teardown occurs before restoring real timers.
- Around line 383-392: Update the “agent tool surface” test to validate the
complete AGENT_API_OPERATIONS allowlist rather than excluding only the currently
known operations. Assert the exact approved operation names, or verify that
every listed operation is read-only with createEvalSuiteOperation as the sole
exception, while preserving the existing in-app gate assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 26ab0c4e-5251-4153-8896-e660a84e4e3c
📒 Files selected for processing (4)
docs/reference/openapi.jsondocs/reference/public-api.mdxmcpjam-inspector/server/routes/v1/__tests__/agent.test.tsmcpjam-inspector/server/routes/v1/agent.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- docs/reference/public-api.mdx
- mcpjam-inspector/server/routes/v1/agent.ts
- docs/reference/openapi.json
1 participant
What
A new public-API terminal for the shared agent engine:
POST /api/v1/projects/:projectId/agentruns ONE assistant turn over a caller-supplied message history and returns the reply, invoked operations, created-resource references (with app deep links), and token usage. First consumer: the MCPJam Slack app (separate repo/PR) — Slack collects the thread, calls this endpoint, posts the reply.Design decisions (from the reviewed plan)
prepareChatV2→resolveTurnRuntime→runUnifiedAssistantTurn(streamSink: "none",persistMode: "caller",approvalMode: "auto-deny"), modeled ondrainAssistantTurn. No forked engine logic.create_eval_suite.run_*/cancel_*/generate_eval_casesare deliberately absent — auto-deny has no interactive fallback, so an unattended turn must not spend eval quota/credits. Runs stay an explicit caller action (POST /eval-runs).projectId; an explicit foreign selector is rejected. The in-appWORKSPACE_OPERATIONSgate (isMcpjamToolId) is untouched — asserted by test.getConvexBearerForRequest) backs both the engine's/streamcalls (JWT-only) and the self-dispatched platform-op calls (keeps agent tool calls out of the caller's per-keysk_rate bucket). Billing rides the pathprojectIdon the hosted rail.anthropic/claude-sonnet-5), docs MCP server with preflight-degrade,skillsSource: none, no tasks seam, no chat persistence.RATE_LIMITED; missing turnTrace →INTERNAL_ERROR; missing backend wiring (OSS installs) →FEATURE_NOT_SUPPORTED.createdResourcesare collected via an execution wrapper before the 24 KB model-facing cap can truncate ids.api_agent_turn_completed(names/counts/durations only — never args/outputs).Security notes
GUEST_ALLOWED_V1_RULESentry).Tests
13 new tests (
agent.test.ts): auth/guest gating, schema limits, deployment guard, delegated-JWT plumbing, docs-degrade, spend-cap → 429 mapping, concurrency cap, project clamp, pre-truncation collector, op-list/gate assertions. Full v1 suite + built-in tools: 247 passed.openapi-driftgreen (newAgenttag + schemas).🤖 Generated with Claude Code
Note
High Risk
New public surface spends hosted model credits, can create eval suites on non-idempotent turns, and relies on project clamping plus delegated JWT for authorization—mistakes in tool scope or retry guidance could cause duplicate resources or cross-tenant issues.
Overview
Adds
POST /api/v1/projects/:projectId/agent, a synchronous public API for one headless assistant turn over caller-ownedmessages, wired through the existingprepareChatV2→resolveTurnRuntime→runUnifiedAssistantTurnpath (streamSink: "none",persistMode: "caller",approvalMode: "auto-deny").The route exposes a curated platform-op tool set (reads plus
create_eval_suiteonly—no run/cancel/generate), hard-clamps every op to the pathprojectId, strips cross-project metadata, and returnsreply,toolCalls,createdResources(with deep links), andusage. Auth uses a delegated Convex JWT for the engine and in-process v1 op calls; guests are denied; hosted-only with caps (4 concurrent turns/org, ~90s, message byte limits) and partial-statedetails.createdResourceson failures.Also documents the Agent tag in OpenAPI/
public-api.mdx, exportscapForModel/toToolErrorfor reuse, addsapi_agent_turn_completedanalytics, and shipsagent.test.tscoverage for gating, limits, concurrency, and the tool adapter.Reviewed by Cursor Bugbot for commit 9d1ff5d. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds a new headless agent endpoint
POST /api/v1/projects/:projectId/agentthat runs one assistant turn on a pinned hosted model and returns the reply, invoked operations, created-resource links, and token usage. First consumer is the MCPJam Slack app; preflight, limits, and retry guidance were tightened.messages. Responds withreply,toolCalls,createdResources, andusage. Not idempotent — dedupe on trigger identity and checkdetails.createdResourcesbefore retrying.create_eval_suite. Run/cancel and generation ops are excluded;approvalMode: "auto-deny".projectId; foreign selectors are rejected. Tool schemas advertiseprojectas optional;otherProjectsmetadata is stripped from results.projectId.anthropic/claude-sonnet-5, static system prompt, docs MCP server with a 5s preflight raced against the turn’s abort signal (bounded, timer disarmed on settle). Authored suites default toanthropic/claude-haiku-4.5.RATE_LIMITED; missingturnTrace→INTERNAL_ERROR; missing hosted wiring →FEATURE_NOT_SUPPORTED. Caller disconnects abort the turn. Failed/timed-out turns includedetails.createdResources.createdResources(e.g., eval suites with app deep links).api_agent_turn_completed. OpenAPI andpublic-api.mdxupdated (Agent tag and schemas; documented500response; byte/concurrency caps). Tests: 21 cases for route, limits, clamp, preflight, disconnect abort, and created-on-failure behavior.Written for commit 9d1ff5d. Summary will update on new commits.