feat: support automations without repositories#836
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds support for automations and sessions without repository targets. It updates shared types, schema, route validation, sandbox/runtime wiring, analytics, and UI labels to carry ChangesNo-repository target support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
352cbb7 to
2c77210
Compare
Co-authored-by: Orca <help@stably.ai>
2c77210 to
21d3712
Compare
Co-authored-by: Orca <help@stably.ai>
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/control-plane/src/session/http/handlers/pull-request.handler.ts (1)
56-74: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate PR creation on repo target before auth lookup, and don't require
base_branch.
SessionPullRequestServicealready falls back torepoInfo.defaultBranch, so this new check turns repo-backed sessions with a nullbase_branchinto false 400s. It should also run before participant/auth resolution so repo-less sessions consistently fail on the intended boundary instead of surfacing an unrelated auth error.Suggested fix
const session = deps.getSession(); if (!session) { return Response.json({ error: "Session not found" }, { status: 404 }); } + if (!session.repo_owner || !session.repo_name) { + return Response.json( + { error: "Pull requests require a repository target" }, + { status: 400 } + ); + } const promptingParticipantResult = await deps.getPromptingParticipantForPR(); if (!promptingParticipantResult.participant) { return Response.json( { error: promptingParticipantResult.error }, @@ - if (!session.repo_owner || !session.repo_name || !session.base_branch) { - return Response.json( - { error: "Pull requests require a repository target" }, - { status: 400 } - ); - } - const result = await deps.createPullRequest({🤖 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/control-plane/src/session/http/handlers/pull-request.handler.ts` around lines 56 - 74, Move the repository-target validation in pull-request.handler’s request flow so it runs before getPromptingParticipantForPR/resolveAuthForPR, and change the guard to only require repo_owner and repo_name. Remove base_branch from the required-session check so SessionPullRequestService can continue falling back to repoInfo.defaultBranch, and keep the existing 400 response for repo-less sessions at that earlier boundary.packages/control-plane/src/routes/slack-notify.ts (1)
88-99: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a no-repo/global denial message on this fallback path.
When
repoScopeis null, this branch is using global Slack settings, but the error still says notifications are disabled “for this repository”. That will mislead users debugging no-repository sessions.🤖 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/control-plane/src/routes/slack-notify.ts` around lines 88 - 99, The fallback path in slack-notify.ts uses global Slack settings when repoScope is null, but the denial message still says the feature is disabled “for this repository.” Update the failureResponse text in this branch to use a no-repo/global wording, and keep the change aligned with the existing resolveSlackSettings, logDenial, and failureResponse flow so users get an accurate message for global sessions.
🧹 Nitpick comments (5)
packages/control-plane/src/sandbox/lifecycle/manager.test.ts (1)
503-513: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the session-id
sandboxIdfallback.This case covers the repo-null payload, but it doesn't verify the new
buildSandboxIdForSession()fallback that switches repo-less sessions tosession.id. Adding that expectation would lock down the core contract change indoSpawn().Suggested test tweak
- const storage = createMockStorage( - createMockSession({ + const session = createMockSession({ repo_owner: null, repo_name: null, repo_id: null, base_branch: null, code_server_enabled: 1, - }), - sandbox - ); + }); + const storage = createMockStorage(session, sandbox); ... expect(provider.createSandbox).toHaveBeenCalledWith( expect.objectContaining({ + sandboxId: expect.stringContaining(`sandbox-${session.id}-`), repoOwner: null, repoName: null, branch: null,🤖 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/control-plane/src/sandbox/lifecycle/manager.test.ts` around lines 503 - 513, The repo-null spawn test in manager.test.ts should also assert the new sandboxId fallback used by doSpawn(). Update the expectation around provider.createSandbox to verify that buildSandboxIdForSession() resolves to session.id when repoOwner, repoName, and branch are null, so the test locks down the repo-less session contract change.packages/control-plane/src/scheduler/durable-object.test.ts (1)
481-500: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that target-resolution failure aborts before session creation.
This test only checks the failed run record. If
/internal/tickcreates a session before marking the run failed, the test still passes and we leak orphan session state. Add a no-session-created assertion here as well.🤖 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/control-plane/src/scheduler/durable-object.test.ts` around lines 481 - 500, The durable-object test for /internal/tick only verifies the failed run record and misses the case where a session could still be created before failure. Update the test around createSchedulerDO and scheduler.fetch to also assert that no session is created when mockCheckRepositoryAccess returns null, using the relevant session-creation mock or assertion in this spec so target-resolution failure is verified to abort before any session state is created.packages/control-plane/src/routes/automations.test.ts (1)
214-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert that the no-repo create path never resolves SCM access.
This case only checks the persisted null repo fields. If
handleCreateAutomation()regressed and still calledresolveRepoOrError()before discarding the result, the test would still pass while reintroducing the dependency this feature is meant to remove. Add an explicitnot.toHaveBeenCalled()assertion for the resolver in this scenario.🤖 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/control-plane/src/routes/automations.test.ts` around lines 214 - 246, The no_repository automation test only verifies persisted null repo fields and could miss an accidental SCM lookup in handleCreateAutomation(). Update the no_repository test case in automations.test.ts to also assert the repo resolver path is not used by checking the resolver mock (the resolveRepoOrError-related mock) was not called. Keep the existing create assertions, and add the negative call assertion in the no_repository scenario so regressions that still resolve SCM access are caught.packages/shared/src/types/index.ts (1)
771-805: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftModel target-specific automation shapes as a discriminated union.
AutomationandCreateAutomationRequestnow represent two mutually exclusive states, but these flat interfaces still allow impossible combinations like{ targetMode: "no_repository", repoOwner: "acme" }or{ targetMode: "fixed_single_repo" }. That pushes the new contract back into runtime checks and test helpers instead of letting TypeScript catch it.As per coding guidelines, "When threading existing fields through new code paths in TypeScript, evaluate whether the existing design (naming, types, units) is correct and fix bad names or units in the same change rather than propagating problems."
🤖 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/shared/src/types/index.ts` around lines 771 - 805, Model Automation and CreateAutomationRequest as discriminated unions keyed by targetMode instead of flat interfaces, so TypeScript can enforce the two valid shapes. Update the Automation and CreateAutomationRequest definitions in the shared types module to split fixed_single_repo from no_repository, making repoOwner/repoName/repoId/baseBranch required only where applicable and absent or null where not. Keep targetMode as the discriminator and adjust any related TriggerConfig typing or dependent fields so impossible combinations are no longer representable.Source: Coding guidelines
packages/control-plane/src/routes/slack-notify.test.ts (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset
getGlobalbetween tests.
vi.clearAllMocks()only clears call history. After Line 428 sets a resolved value here, later cases keep that implementation ifhandleSlackNotifyever starts consultinggetGlobalfor repo-backed sessions, which can turn that regression into a false green.Suggested fix
beforeEach(() => { vi.clearAllMocks(); + integrationStoreMock.getGlobal.mockReset(); sessionFetchMock.mockResolvedValue(new Response("{}", { status: 200 })); vi.stubGlobal("fetch", fetchMock);Also applies to: 420-430
🤖 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/control-plane/src/routes/slack-notify.test.ts` at line 13, Reset the mocked getGlobal between tests so one case’s resolved value does not leak into later cases. Update the slack-notify test setup around getGlobal and the affected scenarios in handleSlackNotify to restore the mock implementation in test teardown or reinitialize it per test, since vi.clearAllMocks only clears calls and not return values.
🤖 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/control-plane/src/db/automation-store.ts`:
- Around line 69-80: The target mode mapping in automation-store is coercing
unknown values into "fixed_single_repo" instead of validating them. Update
automationTargetMode() and the toAutomation() path to reject unsupported
row.target_mode values with an error, matching resolveAutomationTarget(), so
invalid modes are not returned as valid automations; keep bindAutomationInsert()
unchanged and ensure any parsing/validation failure is surfaced from
toAutomation().
In `@packages/control-plane/src/db/session-index.ts`:
- Around line 105-106: Normalize repository fields before persistence in the
session index mapping so blank or whitespace-only values are stored as null
instead of empty strings. Update the logic around session.repoOwner and
session.repoName in the session-index write path to trim and convert blank
values to null, keeping the existing lowercase normalization for real values;
this ensures the downstream no-repo handling works consistently.
In `@packages/control-plane/src/routes/session-create.ts`:
- Around line 42-43: The guard in session-create should distinguish between a
true no-repo request and a partial repo payload. Update the validation around
the repository fields so `NO_REPOSITORY_SESSIONS_AUTOMATION_ONLY_ERROR` is
returned only when both `body.repoOwner` and `body.repoName` are missing, and
when just one is absent, fall through to the existing validation error handling
instead. Use the existing request handling in `session-create` to keep the
repo-backed payload checks consistent.
In `@packages/control-plane/src/sandbox/providers/daytona-provider.ts`:
- Around line 205-207: The Daytona sandbox label generation still emits
openinspect_repo even when config.repoOwner/config.repoName are absent, which
produces malformed repo metadata for repo-less sessions. Update buildLabels() in
the Daytona provider to omit openinspect_repo entirely or special-case it when
REPOSITORY_MODE is "none", and only set it when both repoOwner and repoName are
present so labels stay valid.
In `@packages/control-plane/src/sandbox/providers/vercel/provider.ts`:
- Around line 346-348: The Vercel provider tag construction in buildTags() still
assumes repoOwner and repoName are present, which creates malformed
openinspect_repo values like “null/null” for repo-less sessions. Update
buildTags() to mirror buildEnvVars() by checking REPOSITORY_MODE or the nullable
repo fields and emit a safe non-repo tag value when
config.repoOwner/config.repoName are missing, keeping metadata consistent with
the provider’s “none” mode.
In `@packages/control-plane/src/scheduler/durable-object.ts`:
- Around line 74-80: The Slack target label is currently derived from legacy
repo fields in formatAutomationTargetLabel, which can show stale owner/name
after a target_mode switch to "no_repository". Update the label logic to use
target_mode as the source of truth, matching resolveAutomationTarget, so
repo-less automations always render "No repository" even if repo_owner/repo_name
are still populated. Apply the same fix wherever this formatter is used in the
durable-object scheduler flow so notifications stay consistent with
AutomationRow target_mode.
In
`@packages/control-plane/src/session/http/handlers/session-lifecycle.handler.ts`:
- Around line 21-25: Reject partial repository targets during session
initialization: in session-lifecycle.handler (and the session init path that
persists repo metadata), require repoOwner and repoName to be provided together
or not at all, and do not allow repoId to be stored independently when the repo
target is incomplete. Update the validation/persistence logic so mixed states
are rejected before saving the session, keeping the session row invariant
consistent for downstream handlers.
In `@packages/control-plane/src/session/schema.ts`:
- Around line 389-445: The session table rebuild is not atomic, so a failure
midway can leave the database half-migrated and lose rows; wrap the rebuild
sequence in a transaction inside the migration callback used by
applyMigrations(). Keep the existing logic in the run(sql) migration block, but
execute the DROP/CREATE/INSERT/RENAME steps as one unit so any error rolls back
cleanly instead of leaving session_new or a dropped session table behind.
In `@packages/modal-infra/src/sandbox/manager.py`:
- Around line 63-64: The _repository_mode helper is treating partial repo
metadata as "none", which hides corrupted repository-backed state. Update
_repository_mode to only return "single" when both repo_owner and repo_name are
present, and otherwise reject partial tuples by surfacing an error or explicit
invalid state so create/restore cannot silently lose repository context.
In `@packages/web/src/components/session-header.tsx`:
- Around line 43-47: The session header fallback logic in session-header should
treat null repo fields as a loaded no-repository state instead of staying on
"Loading session...". Update the repoLabel computation around the sessionState /
resolvedRepoOwner / resolvedRepoName branching so it uses the presence of
fallback session data to decide between "No repository" and loading, and make
sure the title fallback reuses that corrected label from the same logic.
In `@packages/web/src/components/sidebar/metadata-section.tsx`:
- Around line 27-30: The MetadataSection component still skips the repository
row when repoOwner and repoName are null, so update its render path to show an
explicit “No repository” state instead of omitting it. Adjust the logic in
MetadataSection to handle repo-less sessions using the new nullable props and
keep the repository row visible with a clear fallback label.
---
Outside diff comments:
In `@packages/control-plane/src/routes/slack-notify.ts`:
- Around line 88-99: The fallback path in slack-notify.ts uses global Slack
settings when repoScope is null, but the denial message still says the feature
is disabled “for this repository.” Update the failureResponse text in this
branch to use a no-repo/global wording, and keep the change aligned with the
existing resolveSlackSettings, logDenial, and failureResponse flow so users get
an accurate message for global sessions.
In `@packages/control-plane/src/session/http/handlers/pull-request.handler.ts`:
- Around line 56-74: Move the repository-target validation in
pull-request.handler’s request flow so it runs before
getPromptingParticipantForPR/resolveAuthForPR, and change the guard to only
require repo_owner and repo_name. Remove base_branch from the required-session
check so SessionPullRequestService can continue falling back to
repoInfo.defaultBranch, and keep the existing 400 response for repo-less
sessions at that earlier boundary.
---
Nitpick comments:
In `@packages/control-plane/src/routes/automations.test.ts`:
- Around line 214-246: The no_repository automation test only verifies persisted
null repo fields and could miss an accidental SCM lookup in
handleCreateAutomation(). Update the no_repository test case in
automations.test.ts to also assert the repo resolver path is not used by
checking the resolver mock (the resolveRepoOrError-related mock) was not called.
Keep the existing create assertions, and add the negative call assertion in the
no_repository scenario so regressions that still resolve SCM access are caught.
In `@packages/control-plane/src/routes/slack-notify.test.ts`:
- Line 13: Reset the mocked getGlobal between tests so one case’s resolved value
does not leak into later cases. Update the slack-notify test setup around
getGlobal and the affected scenarios in handleSlackNotify to restore the mock
implementation in test teardown or reinitialize it per test, since
vi.clearAllMocks only clears calls and not return values.
In `@packages/control-plane/src/sandbox/lifecycle/manager.test.ts`:
- Around line 503-513: The repo-null spawn test in manager.test.ts should also
assert the new sandboxId fallback used by doSpawn(). Update the expectation
around provider.createSandbox to verify that buildSandboxIdForSession() resolves
to session.id when repoOwner, repoName, and branch are null, so the test locks
down the repo-less session contract change.
In `@packages/control-plane/src/scheduler/durable-object.test.ts`:
- Around line 481-500: The durable-object test for /internal/tick only verifies
the failed run record and misses the case where a session could still be created
before failure. Update the test around createSchedulerDO and scheduler.fetch to
also assert that no session is created when mockCheckRepositoryAccess returns
null, using the relevant session-creation mock or assertion in this spec so
target-resolution failure is verified to abort before any session state is
created.
In `@packages/shared/src/types/index.ts`:
- Around line 771-805: Model Automation and CreateAutomationRequest as
discriminated unions keyed by targetMode instead of flat interfaces, so
TypeScript can enforce the two valid shapes. Update the Automation and
CreateAutomationRequest definitions in the shared types module to split
fixed_single_repo from no_repository, making
repoOwner/repoName/repoId/baseBranch required only where applicable and absent
or null where not. Keep targetMode as the discriminator and adjust any related
TriggerConfig typing or dependent fields so impossible combinations are no
longer representable.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4388ed0-1bf8-4071-9bf0-ac1f7845eb28
📒 Files selected for processing (66)
docs/AUTOMATIONS.mdpackages/control-plane/src/automation/target-resolution.test.tspackages/control-plane/src/automation/target-resolution.tspackages/control-plane/src/db/analytics-store.tspackages/control-plane/src/db/automation-store.test.tspackages/control-plane/src/db/automation-store.tspackages/control-plane/src/db/mcp-servers.tspackages/control-plane/src/db/session-index.tspackages/control-plane/src/router.create-session.test.tspackages/control-plane/src/router.spawn-child.test.tspackages/control-plane/src/routes/automations.test.tspackages/control-plane/src/routes/automations.tspackages/control-plane/src/routes/session-child-spawn.tspackages/control-plane/src/routes/session-create.tspackages/control-plane/src/routes/slack-notify.test.tspackages/control-plane/src/routes/slack-notify.tspackages/control-plane/src/sandbox/client.tspackages/control-plane/src/sandbox/lifecycle/manager.test.tspackages/control-plane/src/sandbox/lifecycle/manager.tspackages/control-plane/src/sandbox/provider.tspackages/control-plane/src/sandbox/providers/daytona-provider.tspackages/control-plane/src/sandbox/providers/vercel/provider.tspackages/control-plane/src/sandbox/sandbox-env.tspackages/control-plane/src/scheduler/durable-object.test.tspackages/control-plane/src/scheduler/durable-object.tspackages/control-plane/src/session/create-session-input.tspackages/control-plane/src/session/durable-object.tspackages/control-plane/src/session/http/handlers/child-sessions.handler.tspackages/control-plane/src/session/http/handlers/pull-request.handler.tspackages/control-plane/src/session/http/handlers/sandbox.handler.test.tspackages/control-plane/src/session/http/handlers/sandbox.handler.tspackages/control-plane/src/session/http/handlers/session-lifecycle.handler.tspackages/control-plane/src/session/initialize.tspackages/control-plane/src/session/integration-settings-resolution.tspackages/control-plane/src/session/openai-token-refresh-service.tspackages/control-plane/src/session/pull-request-service.tspackages/control-plane/src/session/repository.tspackages/control-plane/src/session/schema.test.tspackages/control-plane/src/session/schema.tspackages/control-plane/src/session/types.tspackages/control-plane/test/integration/analytics.test.tspackages/control-plane/test/integration/scheduler-slack-events.test.tspackages/control-plane/test/integration/webhooks-slack.test.tspackages/modal-infra/src/sandbox/manager.pypackages/modal-infra/src/web_api.pypackages/modal-infra/tests/test_sandbox_env_vars.pypackages/modal-infra/tests/test_web_api_create_sandbox.pypackages/sandbox-runtime/src/sandbox_runtime/entrypoint.pypackages/sandbox-runtime/src/sandbox_runtime/types.pypackages/sandbox-runtime/tests/test_entrypoint_build_mode.pypackages/sandbox-runtime/tests/test_tool_installation.pypackages/shared/src/triggers/testing.tspackages/shared/src/types/index.tspackages/web/src/app/(app)/automations/[id]/edit/page.tsxpackages/web/src/app/(app)/automations/[id]/page.tsxpackages/web/src/components/automations/automation-form.test.tsxpackages/web/src/components/automations/automation-form.tsxpackages/web/src/components/automations/automations-list.tsxpackages/web/src/components/global-command-menu.tsxpackages/web/src/components/session-header.tsxpackages/web/src/components/session-sidebar.tsxpackages/web/src/components/settings/data-controls-settings.tsxpackages/web/src/components/sidebar/child-sessions-section.tsxpackages/web/src/components/sidebar/metadata-section.tsxpackages/web/src/lib/repo-label.tsterraform/d1/migrations/0029_allow_no_repository_targets.sql
Co-authored-by: Orca <help@stably.ai>
|
Skipping the CodeRabbit nitpicks intentionally.
The actual behavior fixes are committed in d76702a and covered by the final CONTRIBUTING.md checks. |
|
@rubenlangeweg can you include a few screenshots of the UI for this |
Co-authored-by: Orca <help@stably.ai>
|
@ColeMurray attached some screenshots and a recording.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/control-plane/src/sandbox/lifecycle/manager.test.ts (1)
1300-1305: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the timeout fixtures into named
...Msconstants.The inline
10000and11 * 60 * 1000literals make the test harder to scan and drift from the repo’s duration rule for TypeScript. Use named millisecond constants so the units stay explicit in the fixture setup.Suggested change
it("does not explicitly stop providers when the capability is disabled", async () => { + const HEARTBEAT_AGE_MS = 10_000; + const INACTIVITY_AGE_MS = 11 * 60 * 1000; const now = Date.now(); const sandbox = createMockSandbox({ status: "ready", - last_heartbeat: now - 10000, - last_activity: now - 11 * 60 * 1000, + last_heartbeat: now - HEARTBEAT_AGE_MS, + last_activity: now - INACTIVITY_AGE_MS, });As per coding guidelines,
For durations and timeouts: use seconds for Python, milliseconds for TypeScript, encode the unit in variable names (Python: timeout_seconds, TypeScript: timeoutMs, INACTIVITY_TIMEOUT_MS), define each default value exactly once in a named constant, and do not restate literal values in comments.🤖 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/control-plane/src/sandbox/lifecycle/manager.test.ts` around lines 1300 - 1305, The sandbox fixture in manager.test.ts uses inline millisecond durations that should be named constants for clarity and consistency. Replace the literal heartbeat and inactivity values in the createMockSandbox setup with explicit TypeScript millisecond constants (for example, timeoutMs-style names) defined once nearby, and reference those constants in the fixture so the units stay obvious and the duration rule is followed.Source: Coding guidelines
🤖 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 `@packages/control-plane/src/sandbox/lifecycle/manager.test.ts`:
- Around line 1300-1305: The sandbox fixture in manager.test.ts uses inline
millisecond durations that should be named constants for clarity and
consistency. Replace the literal heartbeat and inactivity values in the
createMockSandbox setup with explicit TypeScript millisecond constants (for
example, timeoutMs-style names) defined once nearby, and reference those
constants in the fixture so the units stay obvious and the duration rule is
followed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 820b9974-1bf3-4c7e-9932-63b820ce6df5
📒 Files selected for processing (5)
packages/control-plane/src/sandbox/lifecycle/manager.test.tspackages/control-plane/src/session/durable-object.tspackages/sandbox-runtime/src/sandbox_runtime/entrypoint.pypackages/sandbox-runtime/tests/test_tool_installation.pypackages/shared/src/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/sandbox-runtime/tests/test_tool_installation.py
- packages/shared/src/types/index.ts
- packages/control-plane/src/session/durable-object.ts
- packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py
Co-authored-by: Orca <help@stably.ai>
|
[Automated Review] 🟡 Architecture (non-blocking): consider a discriminated union for A design call worth making consciously before merge — not a blocker. The PR widens Consequences visible in this diff:
Since type Automation = AutomationBase & (
| { targetMode: "fixed_single_repo"; repoOwner: string; repoName: string; baseBranch: string; repoId: number | null }
| { targetMode: "no_repository"; repoOwner: null; repoName: null; baseBranch: null; repoId: null }
);Recommendation: do this now while it's cheap. The PR is framed as the first #833 slice with future dynamic repo discovery — without consolidating here, the next slice repeats the null-handling sweep across the same files. Sessions are a different case: a session's repo-ness is a derived fact, so keeping them nullable + one shared exported Relatedly: |
Co-authored-by: Orca <help@stably.ai>
I also moved I did not add |
Co-authored-by: Orca <help@stably.ai>
| "slack-notify.js": "AGENT_SLACK_NOTIFY_ENABLED", | ||
| } | ||
|
|
||
| AGENT_TOOLS_REQUIRING_REPOSITORY = { |
There was a problem hiding this comment.
these seem like they should be able to run without the repository
There was a problem hiding this comment.
@ColeMurray , for now made a conservative change: status/change tools now install without a repo, while spanw-taks remains repo-gated because the child-spawn control plane path still requires a repo target.
do you want this PR to also support spawning no-repo child sessions end-to-end, or keeping spawn repo-gated the right boundary ?
There was a problem hiding this comment.
if it doesn't introduce several widespread changes, let's proceed updating to allow no-repo child sessions. If it does cause a larger refactor, we can handle in a follow-up PR
There was a problem hiding this comment.
lets finish/address this no-repo child session AFTER the remove targetMode.
Co-authored-by: Orca <help@stably.ai>
|
I think This PR currently introduces repository presence as an automation mode ( I’d rather model this as optional repository context:
That keeps “target” scoped to automation configuration and avoids passing repo mode through session/sandbox/runtime layers. The normalized target rows planned for multi-repo still seem useful, but I don’t think |
Co-authored-by: Orca <help@stably.ai>
|
The recent commit addresses the direction change with focus on supporting
|
## Summary Builds on PR #836 and keeps its commits in this branch history so the original work remains visible and attributed. This adds one follow-up cleanup commit that finishes the repo-less architecture pass we discussed. - replaces the automation launch/target abstraction with a nullable repository-context resolver - enforces repo/base-branch invariants in request schemas, D1 migration, automation storage, and session indexing - allows repo-less child sessions while preventing children from adding or switching repository context - removes repository-mode/no-repository mode naming from Modal and sandbox runtime paths - updates provider/settings copy away from repository-targeting language ## Notes This PR intentionally focuses on the current repo-less automation design from #836. Multi-repo automation/session design from #850 should be handled separately after the repo-less boundary is settled. ## Validation - npm run build -w @open-inspect/shared - npm test -w @open-inspect/shared -- src/types/boundary-schemas.test.ts - npm test -w @open-inspect/control-plane -- src/automation/repository.test.ts src/db/automation-store.test.ts src/db/session-index.test.ts src/session/create-session-input.test.ts - npm test -w @open-inspect/control-plane -- src/routes/automations.test.ts src/router.spawn-child.test.ts src/session/http/handlers/child-sessions.handler.test.ts src/session/http/handlers/pull-request.handler.test.ts src/session/http/handlers/sandbox.handler.test.ts src/session/http/handlers/session-lifecycle.handler.test.ts src/scheduler/durable-object.test.ts - npm test -w @open-inspect/web -- src/components/automations/automation-form.test.tsx - npm run typecheck -w @open-inspect/shared - npm run typecheck -w @open-inspect/control-plane - npm run typecheck -w @open-inspect/web - npm run lint -w @open-inspect/shared - npm run lint -w @open-inspect/control-plane - npm run lint -w @open-inspect/web - cd packages/modal-infra && PYTHONPATH=../sandbox-runtime/src:. pytest tests/test_sandbox_env_vars.py tests/test_web_api_create_sandbox.py -v - cd packages/sandbox-runtime && PYTHONPATH=src pytest tests/test_entrypoint_build_mode.py tests/test_tool_installation.py -v - cd packages/modal-infra && ruff check src/sandbox/manager.py tests/test_sandbox_env_vars.py tests/test_web_api_create_sandbox.py - cd packages/sandbox-runtime && ruff check src/sandbox_runtime/bridge.py src/sandbox_runtime/entrypoint.py tests/test_entrypoint_build_mode.py tests/test_tool_installation.py <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added support for “No repository” sessions and automations with conditional repository configuration and “No repository” display across the UI. * **Bug Fixes** * Improved end-to-end handling of nullable/partial repository fields, including automation/session create & update validation, PR/SCM credential gating, Slack behavior, analytics grouping, and sandbox/no-repo runtime behavior. * **Documentation** * Updated automation setup docs to describe the new optional repository configuration flow. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Ruben Langeweg <rubenlangeweg@gmail.com> Co-authored-by: Orca <help@stably.ai> Co-authored-by: Ruben Langeweg <34856866+rubenlangeweg@users.noreply.github.com>



Summary
Automations can now run without a repository target, so operational workflows can start agent sessions without cloning a code workspace or inventing a placeholder repo. Repository-backed automations still resolve through SCM access before session creation, while no-repository sessions disable repo-only behavior like code-server, child sessions, and PR creation.
Current boundary:
no_repositoryautomations are coordinator sessions only; code investigation or PR work should start a separate repo-bound session after a repository is discovered.Why
This is the first target-resolution slice for #833. It enables operational automations that do not know a repository at trigger time, such as an incident/catalog workflow that first investigates external context, then starts a separate repo-bound session only after the relevant repository is discovered.
What Changed
fixed_single_repo/no_repositorytarget handling at automation creation and scheduling time.0029_allow_no_repository_targets.sql.Below is a recording of the working implementation deployed on CF.
CleanShot.2026-06-27.at.12.44.34.mp4
Validation
npm run typechecknpm run lintnpm testpackages/control-plane: analytics integration test viavitest.integration.config.tspackages/modal-infra: focused pytest and ruff on touched filespackages/sandbox-runtime: focused pytest on touched runtime testsPRAGMA foreign_key_check = 0git diff --checkRefs #833
Made with Orca 🐋
Summary by CodeRabbit
fixed_single_repovsno_repository), including repository configuration UI and consistent “No repository” labeling.