Skip to content

api: answer process routes by a command's process id on guests that mint their own - #1585

Merged
NancyKwongPerplexity merged 2 commits into
mainfrom
native-process-id-older-guests
Nov 9, 2026
Merged

NancyKwongPerplexity merged 2 commits into
mainfrom
native-process-id-older-guests

Conversation

@NancyKwongPerplexity

@NancyKwongPerplexity NancyKwongPerplexity commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

createCommand returns process_id, the command's session id (proc_…), and the data plane sends it to the guest as StartRequest.process_id. Guests within the supported range (>=0.1.0, <0.9.0) that predate assigned ids ignore the field and mint their own id; the data plane then binds the session to the id the guest reported (data_plane_sessions.backend_process_id). On those guests, GET /v1/sandboxes/{id}/processes/{process_id}, :signal and :kill answered 404 for the id createCommand returned, and listProcesses showed only the guest's id. docs/versioning.md requires existing operations to keep working on supported guests.

The process routes now resolve a session-shaped id through that binding:

  • getProcess, signalProcess and killProcess look up the session the id names in this sandbox and, when the data plane bound it to a different guest id, address the guest by that id. getProcess answers under the id that was asked for.
  • listProcesses lists a process bound to a session under the session id, once, in place of the guest's id. The page cursor and order follow the listed ids.
  • Any other id, including a guest's own id and a session id the guest kept, goes to the guest unchanged, so current guests behave as before. ECW starts processes without an assigned id and binds no session, so its processes are unaffected.

The binding is read whether or not the session has expired or been revoked, since the process can outlive it. The data-plane session retention sweep keeps it for as long: it used to delete a session row an hour after the session expired (24 h after create) or ended, so a command running past that answered 404 again. The sweep now skips a row whose backend_process_id is set and differs from its session id, which only guests that mint their own ids produce. Deleting the sandbox removes the row (the foreign key cascades). Current guests bind the session id itself, so their rows are pruned as before. The lookups go through PublicSessionStore, which owns the API's reads of data_plane_sessions: a primary-key read for each session-shaped id on get, signal and kill, and one read per list page only when the guest lists ids that are not session ids.

Retried-start protection: on current guests a repeated start of an assigned id answers AlreadyExists and the data plane reconnects instead of running the command again. Guests that predate assigned ids never answer that. For them, once the binding commits, every later attach reconnects to the bound guest id, as before. Between the start and the binding's commit, only the guest's handling of the start's idempotency key (the session id, sent on every start) keeps an attach on another replica from starting the command again. This PR does not change that; it documents it at initial_process_stream and PendingProcessBinding.

Validation

At this head (rebased on main):

  • New in crates/l40_api/api/src/routes/public/processes/tests.rs, against Postgres and the simulated worker:
    • command_process_ids_reach_processes_of_guests_that_mint_their_own: a guest that mints its own ids. getProcess, :signal and :kill on the createCommand id reach the process (each then reports signaled with the signal sent), and the list shows each command id exactly once and no guest id. It fails on main: the list is ["sim-proc-1000", "sim-proc-1001"].
    • processes_of_guests_that_keep_assigned_ids_answer_as_before: a guest that keeps the assigned id (with and without a committed binding) and a process no session started are listed and addressed by their own ids.
    • Mutation checks, each applied alone: skipping resolution for get/signal/kill, skipping the list rename, answering get under the guest id, leaving signal or kill unresolved, refusing an unbound session id, or dropping an unbound guest process from the list. Each fails one of the two tests.
    • The simulated worker's process table takes an optional assigned id (SimProcessSpec::process_id); its start RPC still mints every id.
  • New in crates/l40_api/cluster/controller/server/src/data_plane_session_retention/tests.rs, against Postgres: sweep_keeps_a_binding_to_a_guest_minted_process_id_until_its_sandbox_goes. Past retention, it keeps an expired session and a closed one that are bound to a guest-minted id, plus a live unbound session. It deletes an expired unbound session and an expired one bound to its own session id. Deleting the sandbox then removes the kept rows. With the new condition replaced by TRUE, the guest-bound rows are deleted and the test fails.
  • cargo nextest run --config 'profile.test.package.pplx-space-api.opt-level=0' -p pplx-space-api --all-features --locked (Postgres and Redis from the nextest setup scripts): 2595 passed, 1 skipped (#[ignore]), no dependency skips.
  • cargo nextest run -p pplx-space-cluster-controller-server -p pplx-space-data-plane-server -p pplx-space-node-api-sim --all-features --locked: 1685 passed.
  • cargo fmt --all -- --check, cargo clippy -p pplx-space-api -p pplx-space-cluster-controller-server -p pplx-space-data-plane-server -p pplx-space-node-api-sim --all-targets --all-features --locked -- -D warnings, and cargo check --workspace --all-targets --locked pass. I did not run the suites of the other crates that use the simulated worker; they compile, and the worker's start behavior is unchanged.
  • Kind, at an earlier head of this PR, before the retention change (64cd810): scripts/tests/smoke-kind-typed.sh native with SPACE_SMOKE_KIND_ADMIN_TOKEN_FILE set passed 47, failed 0 on main (cd07b19) and 47 passed, 0 failed with every SPACE container at that commit. That pass includes ecw.background_process_control on the native surface, which gets, lists and kills a command by its createCommand process id on the current guest. I did not rerun Kind for the retention change: a smoke run cannot reach it, since it takes a session more than an hour past expiry.

Not validated: a guest older than the current one. The Kind seed template builds the guest from this tree, so it keeps assigned ids; running an older guest there means building and publishing a template from an older guest release, which this repository has no recipe for.

Severity and risk

Severity: medium. On supported older guests, the process id the API returns for a command could not be used with any process endpoint, and once retention removed the binding, a long-running command's id stopped working again.

Risk:

  • get, signal and kill add a primary-key read for session-shaped ids; a list adds one read when the guest lists ids that are not session ids. Current guests store a binding equal to the session id, so their answers are unchanged.
  • Retention now keeps one data_plane_sessions row per attached command or terminal on an older guest until its sandbox is deleted, so those rows grow with the sandbox's lifetime rather than a day. Apart from this PR's binding lookups, every read of the table filters out expired rows, so a kept row is invisible to them once it expires. Guest-minted ids are UUIDv7, so a kept binding cannot match a later process. A closed or revoked session on such a guest stays in session lists until its expiry, 24 h after create, rather than for one hour after it ends.
  • No migration. Roll back by reverting; the sweep then prunes the kept rows on its next passes.

AI assistance: Claude Code drafted the code, tests, and this description; the author reviewed them.

@coderabbitai

coderabbitai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Explain Change →

Automatic approval withheld

Assessment Current risk Maximum for approval
Merge risk High Moderate

Why: This change alters how process endpoints answer for older guests and changes which session rows retention deletes. Tests are reported for both, but both areas are contract- and GC-sensitive, so reviewers should confirm compatibility with supported guests and the retention behavior before merging.

Commit: 03c368a499a7.

No actionable comments were generated in the recent review.

Walkthrough

Public process routes translate between session IDs and guest process IDs for listing, retrieval, signaling, and killing. Session-binding lookups include expired and revoked sessions. Retention collection excludes sessions bound to a different backend process ID. The simulator supports assigned process IDs, and integration tests cover guests that mint IDs and guests that retain assigned IDs. Documentation comments describe process-ID recovery behavior for both guest types.

Merge Risk: High · up to 03c36

This change alters how process endpoints answer for older guests and changes which session rows retention deletes. Tests are reported for both, but both areas are contract- and GC-sensitive, so reviewers should confirm compatibility with supported guests and the retention behavior before merging.

Security architecture risk: Low · up to c0355

Process-ID translation remains behind existing authentication and sandbox ownership checks. No introduced authorization bypass was established. Remaining uncertainty concerns older-guest failure recovery and the longer retention of session records.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — In the traced production routes, caller-controlled process IDs can select processes only within an already authorized sandbox. Translation does not select a different sandbox or organization, and no newly reachable cross-tenant operation was established.

Trust Boundaries and Controls

  • observed — The public router retains native authentication and per-operation owner scoping. Process handlers additionally check sandbox ownership before lookup or guest RPC. The existing binding writer locks the session row, accepts an unset or identical binding, and refuses a conflicting process identity.

Resilience and Maintainability Implications

  • observed — The unchanged startup path persists bindings independently of client disconnection. Before a binding commits, cross-replica duplicate-start protection on guests that ignore assigned IDs relies on those guests honoring the session-derived idempotency key. This is a pre-existing dependency, not an introduced vulnerability.

Hardening Proposals

  • proposed — Consider separating durable process bindings from expirable session payloads, or defining field-level cleanup for retained rows. This would preserve process-ID compatibility without necessarily retaining token hashes, adapter state and other session metadata for the entire sandbox lifetime. This is a data-minimization proposal, not an observed disclosure.
Pre-merge checks: Passed 6

Check name Status Explanation
Title check Passed The title starts with the lowercase area api: and a lowercase summary. It accurately describes the main process-route change and has no trailing period, ticket ID, or internal link.
Description Follows The Pr Template Passed The description contains all four required items: Summary, Validation with results, Severity and risk, and an AI assistance line naming the tool and its contributions.
Bug Fixes Name A Regression Test Passed The Validation section names command_process_ids_reach_processes_of_guests_that_mint_their_own and reports that it fails on main and passes with the fix. The diff adds that test and changes the pr…
Weakened Safeguards Are Justified Passed The diff does not weaken an existing test, assertion, lint, CI check, or repository guard. The changed assertion uses the same JSON macro through an import. No review-guidance files changed.
Agent Tooling Runs No New Commands Passed The diff changes Rust application and test files. It does not change agent plugin manifests, skills, commands, or guideline files.
Public-Safe Content Passed The diff, title, and description contain no identified restricted content. The credentials and addresses in the diff are test values.
Full details: Bug Fixes Name A Regression Test

Explanation

The Validation section names command_process_ids_reach_processes_of_guests_that_mint_their_own and reports that it fails on main and passes with the fix. The diff adds that test and changes the process routes.

Recent review info

Automatic approval policy

Commit: 03c368a499a7.

Setting Effective value Source
Automatic approvals Enabled Global configuration
Merge risk Moderate Global configuration
Security risk Not set Global configuration

No overrides matched. The global limits apply.

Not set means no limit was configured after inheritance. Use @coderabbitai configuration to inspect the resolved settings and their sources.

Run configuration

Commits

Reviewing files that changed from the base of the PR and between c0355de and 03c368a.

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 4 remain after this review.

@kumarpallav-pplx
kumarpallav-pplx self-requested a review November 9, 2026 15:29

@shehan-pplx shehan-pplx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed by Shehan's Review Bot

@NancyKwongPerplexity
NancyKwongPerplexity force-pushed the native-process-id-older-guests branch from 64cd810 to c0355de Compare November 9, 2026 16:32
…int their own

createCommand returns the command's session id as its process id, and
the data plane assigns it to the guest process it starts. Supported
guests that predate assigned ids ignore it and mint their own, which the
data plane binds to the session, so on them getProcess, signalProcess
and killProcess answered 404 for the id createCommand returned, and the
list showed only the guest's id.

Resolve a session-shaped id through the session's binding before
addressing the guest, answer getProcess under the id asked for, and list
a bound process under its session id. Other ids, including a session id
the guest kept, pass through unchanged.

On such guests a repeated start is not refused as AlreadyExists; note
where the data plane relies on that, and what holds a repeated start
back there.
Guests that predate assigned process ids mint their own, and the
process routes resolve a command's process id through the session row
that binds it. Retention deleted that row an hour after the session
expired (24 h after create) or ended, after which a long-running
command's process id answered 404. The sweep now keeps a row whose
binding differs from its session id; the sandbox's deletion removes
it. Current guests bind the session id itself, so their rows are
pruned as before.
@NancyKwongPerplexity
NancyKwongPerplexity force-pushed the native-process-id-older-guests branch from c0355de to 03c368a Compare November 9, 2026 16:52
@NancyKwongPerplexity
NancyKwongPerplexity added this pull request to the merge queue Nov 9, 2026
Merged via the queue into main with commit c7c436c Nov 9, 2026
42 checks passed
@NancyKwongPerplexity
NancyKwongPerplexity deleted the native-process-id-older-guests branch November 9, 2026 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants