spec: add ScheduledSession Kind, session sub-resources, and generic proxy surface#1456
Merged
mergify[bot] merged 15 commits intomainfrom Apr 28, 2026
Merged
Conversation
…, and generic proxy surface - Add ScheduledSession to ERD (entity + relationships to Project and Agent) - Add ScheduledSession model section: cron schedule, timezone, enabled flag, session_prompt, last_run_at/next_run_at, trigger semantics, suspend/resume - Add ScheduledSessions to API Reference with full CRUD + suspend/resume/trigger/runs - Add ScheduledSessions CLI table (acpctl scheduled-session list/get/create/update/delete/suspend/resume/trigger/runs) - Expand Sessions API Reference with Group 2 sub-resources: workspace files, pre-upload files (S3), git, repos, operational (clone/model/displayname/workflow/pod-events/oauth/export), runner protocol (interrupt/feedback/capabilities/mcp/status/tasks) - Add Session Operations CLI table for all new sub-resource commands - Add Generic Proxy section listing all backend URLs not natively in ambient-api-server: project config, repo ops, auth integration flows, session runtime, cluster/platform - Update Implementation Coverage Matrix with new rows and 2026-04-24 date Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
✅ Deploy Preview for cheerful-kitten-f556a0 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a project-scoped ScheduledSession kind with cron scheduling and lifecycle actions, implements a backend pre-auth generic proxy, expands Sessions API with many sub-resources and runner-proxy endpoints, adds CLI and SDK surfaces, and registers new plugins and DB migrations in the API server. Changes
Sequence Diagram(s)sequenceDiagram
participant Cron as Cron Scheduler
participant API as API Server (scheduledSessions)
participant DB as Database
participant Agent as Agent / Runner
Note over Cron,API: Scheduled trigger fires
Cron->>API: POST /projects/{id}/scheduled-sessions/{sched_id}/trigger (internal)
API->>DB: Get ScheduledSession by id
DB-->>API: ScheduledSession (enabled / last_run, next_run)
alt active session exists
API-->>DB: Record missed run / update next_run
API-->>Cron: 200 Skipped (active session)
else no active session
API->>Agent: Call agent start endpoint (idempotent) (proxied if remote)
Agent-->>API: 200 Started
API->>DB: Update last_run_at, compute next_run_at
API-->>Cron: 200 Triggered
end
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/internal/design/ambient-model.spec.md`:
- Around line 178-193: Add a "Compatibility / Migration" subsection under the
ScheduledSession model describing the wire-format and lifecycle guarantees:
state that ScheduledSession objects will include fields ID, project_id,
agent_id, name, description, schedule, timezone, enabled (bool, default true),
session_prompt (string, nullable), last_run_at and next_run_at (nullable ISO8601
timestamps), and canonical created_at/updated_at/deleted_at; specify how
proxied/native backends map or omit these fields (e.g., proxies must supply
enabled and session_prompt or return explicit nulls), define versioning/version
header or schema version tag to distinguish old vs new shapes, document SDK/CLI
behavior for absent fields (defaults to enabled=true, session_prompt empty,
timestamps null) and provide a deprecation timeline and migration steps for
clients to handle older responses.
- Around line 865-909: The spec currently allows blanket passthrough for
unmapped backend paths; update the "All backend paths not mapped to a native
`/api/ambient/v1/...` endpoint are forwarded verbatim" section by adding
explicit normative rules: require deny-by-default proxying and an allowlist of
permitted path groups (reference the "Project Configuration (proxied)",
"Repository Operations (proxied)", and "Auth Integration Flows (proxied —
admin)" headings), specify required caller roles/scopes per allowlisted group
(e.g., project: project-admin or project-service scope; repo:
repo-read/repo-write), enumerate blocked/stripped headers and disallowed target
paths (e.g., admin/runner-internal endpoints under Auth Integration Flows),
mandate injection of service credentials only after scope validation, and
require audit logging of proxied requests and authorization decisions; make
these normative sentences so implementations must enforce RBAC, scope checks,
header restrictions, and deny-by-default behavior.
- Around line 309-313: The docs are ambiguous about whether manual triggers
create concurrent runs or follow the idempotent start behavior and which prompt
wins; pick and document one normative contract: (1) decide if POST
/projects/{id}/agents/{agent_id}/start and POST .../trigger are strictly
idempotent (skip when an active Session exists) or if manual triggers may create
concurrent Sessions/runs, and (2) define prompt precedence so that CLI flag
--prompt (<p>) either overrides the stored session_prompt or is ignored when
absent; then update the "Trigger semantics" paragraph, the "Manual trigger"
paragraph, and the CLI doc for the trigger command (the trigger ... [--prompt
<p>] entry) to state the chosen rule clearly, referencing POST
/projects/{id}/agents/{agent_id}/start, POST .../trigger, session_prompt, and
the CLI --prompt flag so SDK/CLI/server implementers have an unambiguous
contract.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7a315f23-d5b2-4079-a215-3e53ab94b4ad
📒 Files selected for processing (1)
docs/internal/design/ambient-model.spec.md
Comment on lines
+178
to
+193
| ScheduledSession { | ||
| string ID PK "KSUID" | ||
| string project_id FK | ||
| string agent_id FK "which Agent to ignite on each trigger" | ||
| string name "human-readable; unique within project" | ||
| string description | ||
| string schedule "cron expression" | ||
| string timezone "IANA timezone; default UTC" | ||
| bool enabled "false = suspended; schedule not evaluated" | ||
| string session_prompt "injected as Session.prompt on each trigger" | ||
| time last_run_at "nullable; wall-clock time of last trigger" | ||
| time next_run_at "nullable; computed from schedule + timezone" | ||
| time created_at | ||
| time updated_at | ||
| time deleted_at | ||
| } |
Contributor
There was a problem hiding this comment.
Missing compatibility/migration contract for ScheduledSession shape and lifecycle semantics.
The spec introduces a new canonical model (enabled, session_prompt, last_run_at, next_run_at) and lifecycle endpoints, but there’s no transition contract for clients when responses come from native vs proxied backends with different field semantics. This can break SDK/CLI parsing and behavior during rollout.
Please add a short “compatibility/migration” subsection defining expected wire format guarantees (or versioning), including field mapping and deprecation timeline.
As per coding guidelines, "Documentation and refactoring suggestions are welcome when substantive."
Also applies to: 846-859
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/internal/design/ambient-model.spec.md` around lines 178 - 193, Add a
"Compatibility / Migration" subsection under the ScheduledSession model
describing the wire-format and lifecycle guarantees: state that ScheduledSession
objects will include fields ID, project_id, agent_id, name, description,
schedule, timezone, enabled (bool, default true), session_prompt (string,
nullable), last_run_at and next_run_at (nullable ISO8601 timestamps), and
canonical created_at/updated_at/deleted_at; specify how proxied/native backends
map or omit these fields (e.g., proxies must supply enabled and session_prompt
or return explicit nulls), define versioning/version header or schema version
tag to distinguish old vs new shapes, document SDK/CLI behavior for absent
fields (defaults to enabled=true, session_prompt empty, timestamps null) and
provide a deprecation timeline and migration steps for clients to handle older
responses.
Comment on lines
+309
to
+313
| **Trigger semantics:** Each trigger calls `POST /projects/{id}/agents/{agent_id}/start`, which is idempotent. If the Agent already has an active Session at trigger time, the trigger is skipped and recorded as a missed run in the runs list. | ||
|
|
||
| **Manual trigger:** `POST .../trigger` ignites the Agent immediately outside the cron schedule, using the same `session_prompt`. Useful for testing or one-off runs. | ||
|
|
||
| **Suspend / Resume:** `POST .../suspend` sets `enabled=false`; `POST .../resume` sets `enabled=true`. These are named convenience actions equivalent to `PATCH {enabled: false|true}`. |
Contributor
There was a problem hiding this comment.
Scheduled trigger contract is internally ambiguous (idempotent start vs immediate trigger vs CLI --prompt).
Line 309 says trigger semantics are idempotent via POST /agents/{agent_id}/start (skip when active session exists), while Line 311 says manual trigger “ignites immediately,” and Line 378 exposes trigger ... [--prompt <p>] although Line 311 says it uses the same session_prompt.
Define a single normative behavior:
- whether manual trigger can create a concurrent run or must reuse idempotent start behavior, and
- precedence rules for prompt (
trigger --promptvs storedsession_prompt).
Without this, SDK/CLI/server implementations will diverge.
As per coding guidelines, "Flag bugs, security vulnerabilities, logic errors, data loss risks, and meaningful refactoring opportunities."
Also applies to: 378-378
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/internal/design/ambient-model.spec.md` around lines 309 - 313, The docs
are ambiguous about whether manual triggers create concurrent runs or follow the
idempotent start behavior and which prompt wins; pick and document one normative
contract: (1) decide if POST /projects/{id}/agents/{agent_id}/start and POST
.../trigger are strictly idempotent (skip when an active Session exists) or if
manual triggers may create concurrent Sessions/runs, and (2) define prompt
precedence so that CLI flag --prompt (<p>) either overrides the stored
session_prompt or is ignored when absent; then update the "Trigger semantics"
paragraph, the "Manual trigger" paragraph, and the CLI doc for the trigger
command (the trigger ... [--prompt <p>] entry) to state the chosen rule clearly,
referencing POST /projects/{id}/agents/{agent_id}/start, POST .../trigger,
session_prompt, and the CLI --prompt flag so SDK/CLI/server implementers have an
unambiguous contract.
Comment on lines
+865
to
+909
| All backend paths not mapped to a native `/api/ambient/v1/...` endpoint are forwarded verbatim to the backend service. The API server authenticates the caller, injects service credentials, then proxies the request — preserving method, path, query string, body, and response status. | ||
|
|
||
| This allows SDK and CLI clients to reach the full backend surface through a single authenticated endpoint without requiring every backend route to be natively implemented in the API server. Routes listed here are candidates for future native spec entries. | ||
|
|
||
| #### Project Configuration (proxied) | ||
|
|
||
| ``` | ||
| GET PUT /api/projects/{p}/permissions | ||
| GET POST DELETE /api/projects/{p}/keys | ||
| GET PUT /api/projects/{p}/mcp-servers | ||
| GET PUT /api/projects/{p}/runner-secrets | ||
| GET PUT /api/projects/{p}/integration-secrets | ||
| GET /api/projects/{p}/secrets | ||
| GET PUT POST DELETE /api/projects/{p}/feature-flags[/{flagName}[/override|/enable|/disable]] | ||
| GET /api/projects/{p}/feature-flags/evaluate/{flagName} | ||
| GET /api/projects/{p}/runner-types | ||
| GET /api/projects/{p}/models | ||
| GET /api/projects/{p}/integration-status | ||
| GET /api/projects/{p}/access | ||
| ``` | ||
|
|
||
| #### Repository Operations (proxied) | ||
|
|
||
| ``` | ||
| GET /api/projects/{p}/repo/tree | ||
| GET /api/projects/{p}/repo/blob | ||
| GET /api/projects/{p}/repo/branches | ||
| GET /api/projects/{p}/repo/seed-status | ||
| POST /api/projects/{p}/repo/seed | ||
| GET POST /api/projects/{p}/users/forks | ||
| ``` | ||
|
|
||
| #### Auth Integration Flows (proxied — admin) | ||
|
|
||
| ``` | ||
| * /api/auth/github/* | ||
| * /api/auth/google/* | ||
| * /api/auth/jira/* | ||
| * /api/auth/gitlab/* | ||
| * /api/auth/gerrit/* | ||
| * /api/auth/coderabbit/* | ||
| * /api/auth/mcp/* | ||
| GET POST /oauth2callback | ||
| GET /oauth2callback/status | ||
| ``` |
Contributor
There was a problem hiding this comment.
Major security gap: generic proxy needs explicit authorization and route allowlist semantics.
Line 865 currently defines blanket passthrough for unmapped backend paths, including sensitive/admin and runner-internal routes (Lines 897-919). This is a privilege-escalation footgun unless the spec mandates per-route RBAC/scope checks and deny-by-default behavior.
Please add explicit normative rules here (e.g., allowlisted path groups only, required roles/scopes per group, blocked headers, and audit logging requirements) so implementations can’t accidentally proxy privileged routes to broad callers.
As per coding guidelines, "Prioritize Critical and Major severity issues. Minimize Minor and Trivial findings."
Also applies to: 911-919
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/internal/design/ambient-model.spec.md` around lines 865 - 909, The spec
currently allows blanket passthrough for unmapped backend paths; update the "All
backend paths not mapped to a native `/api/ambient/v1/...` endpoint are
forwarded verbatim" section by adding explicit normative rules: require
deny-by-default proxying and an allowlist of permitted path groups (reference
the "Project Configuration (proxied)", "Repository Operations (proxied)", and
"Auth Integration Flows (proxied — admin)" headings), specify required caller
roles/scopes per allowlisted group (e.g., project: project-admin or
project-service scope; repo: repo-read/repo-write), enumerate blocked/stripped
headers and disallowed target paths (e.g., admin/runner-internal endpoints under
Auth Integration Flows), mandate injection of service credentials only after
scope validation, and require audit logging of proxied requests and
authorization decisions; make these normative sentences so implementations must
enforce RBAC, scope checks, header restrictions, and deny-by-default behavior.
… ScheduledSessions CLI - Session ERD updated to match actual model.go: adds name, project_id, created_by_user_id, assigned_user_id, parent_session_id, repo_url, repos, workflow_id, llm_model, llm_temperature, llm_max_tokens, timeout, bot_account_name, resource_overrides, environment_variables; corrects triggered_by_user_id → created_by_user_id - ScheduledSessions CLI table: all 9 commands (list/get/create/update/delete/ suspend/resume/trigger/runs) marked ✅ implemented - acpctl apply -f / apply -k corrected to ✅ (was inconsistently 🔲 while coverage matrix already showed ✅) - Coverage matrix (2026-04-28): · Sessions workspace/files/git/repos/operational/runner-protocol → API Server ✅ · ScheduledSessions CRUD + lifecycle → ✅ across API Server, Go SDK, CLI · Generic proxy (all four rows) → API Server ✅ Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…xy, events status
ambient-model.guide.md:
- Runner Pod Addressing: remove stale claim that api-server does not proxy to
runner pods; replace with accurate description of proxyToRunner() in
plugins/sessions/handler.go
- Lessons Learned: add "Generic Proxy Is a Pre-Auth Middleware, Not a Route"
rule — RegisterPreAuthMiddleware is required for non-/api/ambient/ paths
because RegisterRoutes only receives the /api/ambient/v1 subrouter
control-plane.spec.md:
- API Server Proxy GET /sessions/{id}/events: update status from 🔲 planned to
✅ implemented; cite plugins/sessions/plugin.go → StreamRunnerEvents
- Add Generic Backend Proxy section: describes plugins/proxy/plugin.go,
implementation via RegisterPreAuthMiddleware, BACKEND_URL env var, status ✅
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…trigger/runs
New plugin at plugins/scheduledSessions/ implementing the ScheduledSession Kind:
- model.go: ScheduledSession struct with schedule, timezone, enabled,
session_prompt, last_run_at, next_run_at, agent_id, project_id
- migration.go: Gormigrate table creation
- dao.go / mock_service.go: DAO interface + in-memory mock
- service.go: CRUD + suspend/resume/trigger/runs business logic
- handler.go: HTTP handlers for all 9 operations
- presenter.go: model ↔ OpenAPI response conversion
- plugin.go: route registration under /projects/{id}/scheduled-sessions
- handler_test.go: unit tests covering all handlers
Routes registered:
GET/POST /projects/{id}/scheduled-sessions
GET/PATCH/DELETE /projects/{id}/scheduled-sessions/{sched_id}
POST .../suspend .../resume .../trigger
GET .../runs
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ational sub-resource handlers
13 new handler methods in plugins/sessions/handler.go:
Runner-proxy sub-resources (proxy to runner pod; stub when unavailable):
- WorkspaceList: GET /sessions/{id}/workspace → {"files":[]} stub
- WorkspaceFile: GET/PUT/DELETE /sessions/{id}/workspace/{path} → 503 if no runner
- FilesList: GET /sessions/{id}/files → {"files":[]} stub
- FilesFile: PUT/DELETE /sessions/{id}/files/{path} → 503 if no runner
- GitStatus: GET /sessions/{id}/git/status → empty stub; proxy when runner present
- GitConfigureRemote: POST /sessions/{id}/git/configure-remote → 503 if no runner
- GitBranches: GET /sessions/{id}/git/branches → [] stub
- ReposStatus: GET /sessions/{id}/repos/status → [] stub
- PodEvents: GET /sessions/{id}/pod-events → always [] (K8s-native; no runner needed)
Operational sub-resources (native — no runner required):
- PatchDisplayName: PATCH /sessions/{id}/displayname — validates non-empty; persists via Replace()
- WorkflowMetadata: GET /sessions/{id}/workflow/metadata — parses WorkflowId JSON
- OAuthProviderURL: GET /sessions/{id}/oauth/{provider}/url — 501 Not Implemented
- ExportSession: GET /sessions/{id}/export — {"session":…,"export_at":…,"version":"1"}
Also adds 16 routes to plugin.go (wildcard paths use {path:.*} gorilla mux syntax).
New test files (handlerunit/):
- handler_runner_proxy_test.go: 14 tests — no-runner stubs, 404 on missing session
- handler_operational_test.go: 9 tests — displayname, workflow metadata, oauth, export
- handler_subresource_test.go: updated setupFullRouter with all 16 new routes
All 55 handlerunit tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New plugin at plugins/proxy/ that forwards any request not matching /api/ambient/* to BACKEND_URL (default http://localhost:8080): - plugin.go: pre-auth middleware registered via pkgserver.RegisterPreAuthMiddleware; preserves method, path, query string, all headers (including Authorization), and request/response body verbatim; returns 502 if backend unreachable - proxy_test.go: 10 tests — native paths pass through, non-native paths proxy to mock backend, 502 on backend down, method/headers/body/query preserved, response headers copied back - isNativePath(): exempts /api/ambient/*, /api/ambient, /metrics, /favicon.ico cmd/ambient-api-server/main.go: add blank import to activate proxy init(). RegisterPreAuthMiddleware is required because plugin RegisterRoutes callbacks only receive the /api/ambient/v1 subrouter; paths outside that prefix cannot be intercepted via route registration. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New file: components/ambient-sdk/go-sdk/client/scheduled_session_api.go ScheduledSessionAPI struct with project-scoped basePath and 10 methods: - List(ctx, projectID, limit) → ScheduledSessionList - Get(ctx, projectID, schedID) → ScheduledSession - GetByName(ctx, projectID, name) → ScheduledSession (scans list, matches by name) - Create(ctx, projectID, req) → ScheduledSession - Update(ctx, projectID, schedID, patch) → ScheduledSession - Delete(ctx, projectID, schedID) → error - Suspend(ctx, projectID, schedID) → error - Resume(ctx, projectID, schedID) → error - Trigger(ctx, projectID, schedID) → error - Runs(ctx, projectID, schedID) → SessionList Registered on Client as client.ScheduledSessions() accessor. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
New file: components/ambient-cli/cmd/acpctl/scheduledsession/cmd.go Implements acpctl scheduled-session with subcommands: - list — table with ID, NAME, SCHEDULE, TIMEZONE, ENABLED, NEXT RUN - get — single record by name or ID; -o json supported - create — --name, --agent-id, --schedule required; --timezone, --prompt, --description optional - update — any field optional; name-or-ID resolver tries Get then GetByName fallback - delete — --confirm required - suspend — sets enabled=false - resume — sets enabled=true - trigger — immediate one-off ignite outside cron schedule - runs — lists Sessions triggered by this schedule resolveScheduledSession() helper tries direct ID lookup first, falls back to GetByName() so users can pass human names without knowing internal IDs. resolveProject() reads project from config when --project-id flag is omitted. cmd/acpctl/main.go: register scheduledsession.Cmd between agent and credential. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Merge Queue Status
This pull request spent 15 seconds in the queue, including 2 seconds running CI. Required conditions to merge |
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
1 participant
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
New Kind:
ScheduledSession— added to ERD, model definition, API Reference (/projects/{id}/scheduled-sessions), CLI table, and coverage matrix. Covers CRUD,suspend/resume/trigger, andrunshistory. Linked to bothProjectandAgentin the data model.Session sub-resources (Group 2) — expanded the Sessions API Reference with all operational sub-resource endpoints currently only reachable via the frontend: workspace files, pre-upload files (S3), git status/remote/branches, repos, clone, model switch, display name, workflow, pod events, OAuth URL, export, and runner protocol endpoints (interrupt, feedback, capabilities, MCP status, background tasks). Matching
acpctl session *CLI commands added.Generic proxy surface — added a new spec section documenting all backend URLs not natively implemented in the ambient-api-server that require passthrough proxying: project config (permissions, keys, MCP servers, secrets, feature flags), repository operations, auth integration flows (GitHub/GitLab/Google/Jira/Gerrit/CodeRabbit/MCP), session runtime/runner-internal endpoints, and cluster/platform endpoints. These are tagged as candidates for future native spec entries.
Coverage matrix updated with new rows for all of the above.
Intent
This is a spec-only change — no implementation. The goal is to define the desired state so that:
ScheduledSessionnatively/api/ambient/v1/sessions/{id}/...Test plan
/api/ambient/v1/sessions/{id}conventionsroutes.goacpctlconventions🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Generic Proxy
Documentation