Skip to content

feat(api): Stage I owner-only views and stateless Responses - #382

Open
hanakannzashi wants to merge 5 commits into
mainfrom
codex/issue-379-stateless-surfaces
Open

feat(api): Stage I owner-only views and stateless Responses#382
hanakannzashi wants to merge 5 commits into
mainfrom
codex/issue-379-stateless-surfaces

Conversation

@hanakannzashi

@hanakannzashi hanakannzashi commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Closes #386
Closes #387
Closes #388
Closes #389

Summary

This PR implements the Stage I owner-only read contract in #385. It preserves the existing authenticated owner views needed for migration/export, disables ordinary Conversation/File and all sharing routes, and keeps the Responses proxy stateless.

Stage I API contract

This table covers the Private Chat stateful surface only. Other inference APIs remain outside this migration scope.

Surface Method and path Auth Stage I treatment
Stateless Responses POST /v1/responses Existing dual-auth, subscription, and rate-limit boundary Retained. Every response, including local middleware and validation errors, is Cache-Control: no-store. A non-empty body must be an identity-encoded JSON object. Chat rejects store: true, non-null conversation, non-null previous_response_id, and background: true; it normalizes valid JSON requests to store: false. It does not inject author metadata, perform Conversation ACL/tracking, execute tools, or run an agent loop.
Client-managed functions tools: [{type:"function"}] and replayed input items Same as Responses Retained. After existing request normalization, Chat does not turn these items into another transcript shape, reorder them, or execute them. The caller supplies prior context and matching function-call output on the next request.
Responses built-ins / remote MCP Built-in or remote-MCP tool/input shapes submitted to /v1/responses Same as Responses Not implemented by Chat. Chat forwards the shape to Cloud; Cloud applies its stateless Responses validation and returns its normal 400 for unsupported capabilities. This does not affect the separate root MCP proxy below.
Owner Conversation views GET /v1/conversations
GET /v1/conversations/{conversation_id}
GET /v1/conversations/{conversation_id}/items
Session auth; current user must own the record Temporarily retained for migration/export. All responses are Cache-Control: no-store.
Owner File views GET /v1/files
GET /v1/files/{file_id}
GET /v1/files/{file_id}/content
Session auth; existing caller/ownership checks Temporarily retained for migration/export. All responses are Cache-Control: no-store.
Ordinary Conversation/File operations Every other method/path under /v1/conversations and /v1/files, including create, update, browser-facing delete, item creation, upload, pin/unpin, archive/unarchive, clone, external /v1/conversations/batch, unsupported methods, and unknown descendants Session auth Retired. An authenticated request returns 410 Gone with Cache-Control: no-store; it does not mutate local Private Chat state or forward a Cloud mutation.
Sharing Every method, including GET, under /v1/conversations/{conversation_id}/shares and descendants, /v1/share-groups and descendants, and /v1/shared-with-me and descendants Session auth Retired. Authenticated requests return 410 Gone with Cache-Control: no-store. Optional-auth/public shared-Conversation reads are not mounted.
Account deletion exception DELETE /v1/users/me Existing session-authenticated user route Retained unchanged. On acceptance it creates the existing asynchronous deletion job; its worker uses Cloud #943's retained API-key/workspace-scoped DELETE /v1/conversations/{id} and DELETE /v1/files/{id} before local finalization. This is not a retained browser-facing Conversation/File proxy DELETE route.
Separate MCP proxy POST /mcp Existing dual-auth boundary Unchanged and out of this stateful-surface migration. It is a standalone MCP proxy, not a Responses builtin/remote-MCP tool.
Other inference APIs /v1/chat/completions, /v1/images/*, model/signature routes Existing boundaries Unchanged and out of scope.

Out of scope

No schema/migration/data cleanup, no new export API, and no removal of temporary owner-view wiring. #380 tracks account-deletion compatibility; final route/runtime removal remains Stage III work under #377 and #390#392.

Validation

  • GitHub CI passed on the current 09902665 head: Test Suite, Rust CodeQL, cargo deny, and CodeQL actions
  • cargo fmt --all -- --check
  • git diff --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --lib --bins --features test
  • fresh isolated Docker PostgreSQL: Stage I targeted integration 19/19; full cargo test --features test -- --test-threads=1 passed (one intentional real-agent test ignored)

@hanakannzashi
hanakannzashi requested a review from a team as a code owner August 20, 2026 03:21
@ironloopai

ironloopai Bot commented Aug 20, 2026

Copy link
Copy Markdown

🧭 IronLoop Run · Review

This comment updates in place as the Run moves through its stages.

🟩 Final result · Completed

🟨 Queued🟦 Working🟦 Posting results🟩 Completed

Automatic trigger · attempt 1 of 3 · completed in 1m 2s

IronLoop completed the review and posted it to GitHub.

🔗 Result

Open submitted review →

Run details

Run: c7095235-fe44-429d-92ac-9750953d2312
Base: main at 686d724
Head: codex/issue-379-stateless-surfaces at 8742d3e
Created: 2026-08-20 03:26 UTC
Updated: 2026-08-20 03:27 UTC

@claude

claude Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review: feat(api): retire stateful chat surfaces

Solid execution of #379 — the retirement boundary, the no-store layering (outermost map_response, so middleware rejections are covered too), and the OpenAPI pruning all look right. Three things I would fix before merge.

1. Stateless enforcement is skipped whenever the body does not parse as JSON — crates/api/src/routes/api.rs:2591

validate_stateless_response_body and the store: false normalization both run only inside if let Some(body) = body_json. When serde_json::from_slice fails, or the root is not an object, the original bytes are forwarded verbatim (api.rs:2649-2651).

That is reachable today: extract_body_bytes (api.rs:5155) never decompresses the request body, and ResponseService::forward_request (crates/services/src/response/service.rs:118-136) forwards every client header except authorization/host/cookie/x-org-id/x-workspace-id — including content-encoding. So:

POST /v1/responses
Content-Encoding: gzip
<gzip({"model":"...","store":true,"conversation":"conv_x","previous_response_id":"resp_y"})>

→ local JSON parse fails → no 400, no store: false rewrite → the stateful request reaches Cloud API intact. This handler is the only enforcement point for the core contract of this PR (a normal Responses request no longer becomes store: true), and that is a privacy contract, not just an API nicety.

Fix — Responses only accepts a JSON object anyway, so fail closed:

let Some(mut body) = body_json else {
    return Err(stateless_responses_bad_request(
        "The Responses API requires an uncompressed JSON object body.",
    ));
};
// plus the same 400 when !body.is_object()

(Delegating the empty-body case upstream is fine; the gap is non-empty-but-unparsed.)

2. require_approval object form gets a false 400 — api.rs:558-567

Some("mcp") if tool.get("require_approval").and_then(Value::as_str) != Some("never") => Err(...)

require_approval is "always" | "never" | { "never": { "tool_names": [...] }, "always": {...} }. A config that grants blanket no-approval through the object form ({"never": {"tool_names": ["search"]}}) is a legitimate stateless MCP setup and is rejected here. Accept an object whose only populated key is never:

fn mcp_never_requires_approval(tool: &serde_json::Value) -> bool {
    match tool.get("require_approval") {
        Some(v) if v.as_str() == Some("never") => true,
        Some(serde_json::Value::Object(o)) => o.contains_key("never") && !o.contains_key("always"),
        _ => false,
    }
}

3. #![allow(dead_code)] is module-wide — api.rs:4

The inner attribute silences dead-code detection for all ~6k lines of routes/api.rs, including code added after this PR, for as long as #381 takes to land. cargo clippy -- -D warnings will stop catching newly orphaned code in the busiest module in the crate. Scope it to what is actually dormant: per-item #[allow(dead_code)] on the retained legacy handlers/types, or move them into a #[allow(dead_code)] mod legacy_stateful;.

Nits

  • validate_stateless_response_body runs twice per request — once at api.rs:2592 and again inside normalize_stateless_response_body (api.rs:579). Drop the earlier call.
  • Retired routes still run auth_middleware (a DB session lookup) before returning 410, so stale clients polling /v1/conversations keep generating DB load on a permanently dead path. Preserving the auth boundary is a deliberate call — just flagging the cost.
  • The PR notes integration tests could not be run locally. The router-shape changes (new {*path} catch-alls merged with the optional-auth {conversation_id} routes) are only exercised at router-construction time inside those tests, so please confirm the CI integration job is green before merge.

⚠️

@ironloopai ironloopai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔍 IronLoop review

The stateless proxy mostly implements the retirement boundary, but it unnecessarily removes stateless function calling.

Findings: 🟠 Medium 1

🟠 Medium · Allow client-managed function calling

Inline on crates/api/src/routes/api.rs:428. See the inline comment for details.

Validation

  • Stateless boundary inspection — Traced request validation and normalization through the Responses proxy and confirmed that function definitions and their client-managed outputs are rejected before reaching Cloud API.
Review details
  • Run: c7095235-fe44-429d-92ac-9750953d2312
  • Workflow: Review
  • Attempts: 1

Comment thread crates/api/src/routes/api.rs Outdated
@hanakannzashi

Copy link
Copy Markdown
Contributor Author

Follow-up fixes are in 34e9024.

  • Replaced conflicting wildcard retirement routes with nested-router fallbacks. Exact public Conversation GET routes still merge through optional auth; all other retired paths, trailing slashes, and unknown descendants stay behind session auth and return local 410 with Cache-Control: no-store.
  • Responses now fails closed for every non-empty body unless it is an uncompressed JSON object. identity content encoding remains valid; gzip/other encodings, malformed JSON, and non-object JSON receive a local 400 + no-store and are not forwarded.
  • MCP granular approval objects and function/function-call continuations intentionally remain rejected: Cloud #943’s current stateless contract only supports simple require_approval: "never" and does not support continuation. Chat API should not diverge from that contract.
  • Removed the file-wide dead_code suppression. Only retained legacy handlers/helpers are individually marked pending [Epic 953][Stage III][chat-api] Remove stateful runtime wiring after the read window #381 cleanup.

Validation: cargo fmt --all -- --check; cargo check -p api --features test; cargo test -p api --lib --features test (133 passed); cargo clippy -p api --all-targets --features test -- -D warnings. The PostgreSQL-backed integration tests compile but cannot start locally because the local postgres account fails password authentication during migration bootstrap; CI remains the full integration check.

@hanakannzashi

Copy link
Copy Markdown
Contributor Author

Fixed the failed CI fixture in 7a15469: the test now establishes an active subscription before asserting the handler’s local 400 response. Production authentication and subscription middleware ordering are unchanged.

@hanakannzashi hanakannzashi changed the title feat(api): retire stateful chat surfaces feat(api): Stage I read-only stateful proxy surfaces Aug 21, 2026
@hanakannzashi

hanakannzashi commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Stage I clarification — owner-only views

The Stage I read window retains only authenticated owner Conversation and File views. Sharing is not an export dependency: the entire sharing namespace, including its GET routes, and optional-auth/public shared Conversation reads are 410 Gone with Cache-Control: no-store.

This PR therefore preserves the three Conversation and three File owner views, blocks all writes and sharing routes, and keeps Responses stateless without author injection or local tracking. Client-managed custom function calls and matching function_call_output replay pass through unchanged; Cloud API owns built-in/MCP tool validation.

Final removal of the remaining owner views and wiring is deliberately deferred to Stage III.

@hanakannzashi hanakannzashi changed the title feat(api): Stage I read-only stateful proxy surfaces feat(api): Stage I owner-only views and stateless Responses Aug 21, 2026
@hanakannzashi

Copy link
Copy Markdown
Contributor Author

Update: final head 9bf6ab0 supersedes the earlier all-410/function-rejection discussion. Client-managed function and matching function_call_output replay now pass through unchanged to Cloud; unsupported built-in/MCP tools are validated by Cloud. GitHub Tests, Rust CodeQL, cargo deny, and CodeQL actions are green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant