feat(openshell): add operator capacity safeguards

[dimakis]

Summary

• add authenticated, verified OpenShell inventory, capacity, and lifecycle audit APIs with honest partial/unavailable states
• add separate Podman usage/reclaimable and authoritative filesystem free/total collection plus serialized hysteretic create admission
• preserve feat(openshell): checkpoint and clean up idle sandboxes safely #503's identity-fenced lifecycle preview/confirm semantics while recording sanitized, bounded operator actions

Safety

• new physical sandbox creates fail closed under unavailable or hard capacity pressure; existing reattach/start/restore/checkpoint-stop/consented deletion paths remain available
• provider and lifecycle diagnostics exposed through operator inventory use fixed error codes; raw provider output is not returned or logged
• malformed lifecycle mutation attempts are audited, and the durable audit is capped at 500 entries
• no Podman prune, lifecycle enablement, deployment, or live sandbox action
• Vertex-like capabilities remain explicitly unsupported until a dedicated adapter exists

Validation

• npm test -- --maxWorkers=4 (306 files; 4,371 passed, 10 skipped)
• npm run build:all
• npm run lint (0 errors; 4 existing warnings)
• npm run format:check
• focused lifecycle/routes validation (130 passed)

Integration

Rebased onto current main after #503 merged, preserving the newer authenticated lifecycle endpoints and lifecycle-disabled startup behavior. The pinned image checkpoint helper alignment and live Vertex acceptance remain separate gates.

[dimakis]

Centaur Review

Found 3 issue(s) (3 warning).

server/openshell-capacity.ts

Capacity collection and lifecycle safeguards have correctness gaps that should be resolved before relying on these operator endpoints.

• 🟡 bugs (L85): The collector accepts only numeric JSON fields, but podman system df --format json emits raw size fields as unit-bearing strings (the existing lifecycle observability collector explicitly parses values such as 100B). Consequently real Podman usage/reclaimable metrics are always marked unavailable, even though they are available; parse RawSize/RawReclaimable byte strings here as well. [fixable]

server/openshell-lifecycle-controller.ts

Capacity collection and lifecycle safeguards have correctness gaps that should be resolved before relying on these operator endpoints.

• 🟡 bugs (L336): openShellLifecycleCapability() only changes inventory labels; lifecycle preview/confirm still use the generic checkpoint/stop/delete adapters for every API-route record. Thus a google-/anthropic-Vertex record can be reported as unsupported yet an authenticated caller can invoke lifecycle actions directly. Gate the production adapters/service on the capability, so unsupported providers are actually action-disabled. [fixable]

server/app.ts

Capacity collection and lifecycle safeguards have correctness gaps that should be resolved before relying on these operator endpoints.

• 🟡 bugs (L825): Successful retention-consent audit entries always persist sandboxId: null and generation: null, despite the lifecycle record being available and the stated audit contract requiring target identity and generation for every action. Capture the record identity/generation (with the post-consent generation as appropriate) before writing the audit entry. [fixable]

[dimakis]

Centaur Review

Found 2 issue(s) (2 warning).

server/openshell-capacity.ts

Capacity threshold validation and create-time serialization leave the new safeguard bypassable or overly restrictive.

• 🟡 bugs (L36): The ordering validation omits recoverFreePercent < warningFreePercent. For example warning=20, recover=30, hard=10 is accepted despite the documented required ordering, and keeps the hard-stop circuit active through 20–30% free space. Reject recovery values greater than or equal to warning too. [fixable]

server/openshell-runtime.ts

Capacity threshold validation and create-time serialization leave the new safeguard bypassable or overly restrictive.

• 🟡 unsafe_assumptions (L599): Admission releases its serialization lock as soon as the capacity sample passes, before this physical create runs. Concurrent conversations can all observe sufficient free space and then create simultaneously, defeating the stated serialized capacity safeguard. Hold the global admission/reservation through the create (or recheck under a lock immediately before each create); the current racing test only races admission calls, not creates. [fixable]

[dimakis]

🟡 bugs: The ordering validation omits recoverFreePercent < warningFreePercent. For example warning=20, recover=30, hard=10 is accepted despite the documented required ordering, and keeps the hard-stop circuit active through 20–30% free space. Reject recovery values greater than or equal to warning too. [fixable]

[dimakis]

🟡 unsafe_assumptions: Admission releases its serialization lock as soon as the capacity sample passes, before this physical create runs. Concurrent conversations can all observe sufficient free space and then create simultaneously, defeating the stated serialized capacity safeguard. Hold the global admission/reservation through the create (or recheck under a lock immediately before each create); the current racing test only races admission calls, not creates. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s) (3 warning).

server/openshell-capacity.ts

Capacity safeguards are broadly wired in, but the default filesystem target and stateful unsynchronized status collection can make admission unreliable, and legacy inventory reports are misleading.

• 🟡 bugs (L76): When no capacity path is configured, this runs df on /. Mitzo is a host launchd service and Podman sandboxes may consume a Podman VM filesystem, so host-root free space can remain healthy while the actual sandbox storage is full. The admission check can therefore permit creates precisely when it should hard-stop. Require/configure and document an authoritative Podman storage/VM path, or collect the VM filesystem directly. [fixable]
• 🟡 unsafe_assumptions (L144): snapshot() mutates the shared hard-stop latch but is also called by the unaffiliated capacity status endpoint. It does not participate in tail serialization, so concurrent status polling can clear or set the latch while a create admission is being evaluated, defeating the claimed serialized hysteresis behavior. Serialize all state-mutating snapshots or make status collection read-only. [fixable]

server/openshell-lifecycle-controller.ts

Capacity safeguards are broadly wired in, but the default filesystem target and stateful unsynchronized status collection can make admission unreliable, and legacy inventory reports are misleading.

• 🟡 bugs (L119): Legacy records without identity are emitted as unavailable rows but no unavailable scope is added. The response then reports available: false, partial: false, scopes: [] (and the route returns 200 merely because a row exists), which hides the unavailable collection state from the operations UI. Add an unavailable scope or set partial/availability consistently for these records. [fixable]

[dimakis]

🟡 bugs: When no capacity path is configured, this runs df on /. Mitzo is a host launchd service and Podman sandboxes may consume a Podman VM filesystem, so host-root free space can remain healthy while the actual sandbox storage is full. The admission check can therefore permit creates precisely when it should hard-stop. Require/configure and document an authoritative Podman storage/VM path, or collect the VM filesystem directly. [fixable]

[dimakis]

🟡 unsafe_assumptions: snapshot() mutates the shared hard-stop latch but is also called by the unaffiliated capacity status endpoint. It does not participate in tail serialization, so concurrent status polling can clear or set the latch while a create admission is being evaluated, defeating the claimed serialized hysteresis behavior. Serialize all state-mutating snapshots or make status collection read-only. [fixable]

[dimakis]

🟡 bugs: Legacy records without identity are emitted as unavailable rows but no unavailable scope is added. The response then reports available: false, partial: false, scopes: [] (and the route returns 200 merely because a row exists), which hides the unavailable collection state from the operations UI. Add an unavailable scope or set partial/availability consistently for these records. [fixable]

[dimakis]

Centaur Review

Found 2 issue(s) (2 warning).

server/openshell-lifecycle.ts

Capacity admission is well-covered, but the lifecycle audit is neither durably bounded nor complete for validation failures.

• 🟡 bugs (L210): The audit is only bounded when read (listAudit limits its SELECT); appendAudit never prunes old rows. Authenticated callers can generate unbounded durable audit data, contradicting the documented bounded audit and eventually growing the lifecycle DB indefinitely. [fixable]

server/app.ts

Capacity admission is well-covered, but the lifecycle audit is neither durably bounded nor complete for validation failures.

• 🟡 regressions (L784): Malformed confirm requests return before recording an audit entry; the analogous invalid retention-consent request at line 823 does too. This violates the new operations API contract that all lifecycle action attempts are audited, leaving rejected operator attempts without durable attribution. [fixable]

[dimakis]

Centaur Review

Found 1 issue(s) (1 warning).

server/openshell-runtime.ts

Capacity admission is generally well-covered, but a create-vs-reattach race can incorrectly block an existing sandbox.

• 🟡 regressions (L599): Capacity admission is taken after the initial get result has already established !sandbox, but the sandbox is not re-read once the serialized reservation is acquired. If another process creates the same sandbox while this caller waits, and that create drops free space below the hard threshold, this caller is rejected instead of treating the now-existing sandbox as a reattach (despite the stated guarantee that reattach remains available). Re-check get after acquiring the reservation, and only issue/create-charge the command if it is still absent. [fixable]

[dimakis]

🟡 regressions: Capacity admission is taken after the initial get result has already established !sandbox, but the sandbox is not re-read once the serialized reservation is acquired. If another process creates the same sandbox while this caller waits, and that create drops free space below the hard threshold, this caller is rejected instead of treating the now-existing sandbox as a reattach (despite the stated guarantee that reattach remains available). Re-check get after acquiring the reservation, and only issue/create-charge the command if it is still absent. [fixable]

[dimakis]

Centaur Review

Found 1 issue(s) (1 warning).

server/openshell-lifecycle-controller.ts

Capacity admission and lifecycle safeguards are generally well covered, but the new inventory endpoint can expose unsanitized provider diagnostics.

• 🟡 unsafe_assumptions (L45): sanitizeLifecycleError only redacts path- and URL-shaped substrings, then inventory returns its output as scopes[].error and lastFailure. Provider/CLI diagnostics can contain bearer tokens, grant IDs, response bodies, or other sensitive values without either shape, contradicting the endpoint's stated no-raw-provider-diagnostics contract. Return a fixed error code/message (and keep detailed diagnostics server-side) instead. [fixable]

[dimakis]

🟡 unsafe_assumptions: sanitizeLifecycleError only redacts path- and URL-shaped substrings, then inventory returns its output as scopes[].error and lastFailure. Provider/CLI diagnostics can contain bearer tokens, grant IDs, response bodies, or other sensitive values without either shape, contradicting the endpoint's stated no-raw-provider-diagnostics contract. Return a fixed error code/message (and keep detailed diagnostics server-side) instead. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s) (3 warning).

server/openshell-lifecycle-controller.ts

The safeguards are directionally sound, but inventory completeness, diagnostic redaction, and rollout configuration need correction before merge.

• 🟡 bugs (L139): Inventory scopes are seeded only from lifecycle records, so a provider with no record is never queried. Consequently freshly orphaned sandboxes (for example, a create followed by a crash before provisional registration) and pre-lifecycle sandboxes cannot appear as orphaned; with no records the endpoint incorrectly returns an unavailable empty inventory. Seed collection from sources.accountProviders() as the existing phase-count inventory does. [fixable]

server/app.ts

The safeguards are directionally sound, but inventory completeness, diagnostic redaction, and rollout configuration need correction before merge.

• 🟡 unsafe_assumptions (L685): lifecycleError only removes paths and URLs, but provider/CLI errors can contain bearer tokens, grant IDs, or raw response bodies. Such text is returned to clients and durably written to the new audit table, violating the documented sanitized-error boundary. Use stable error codes or an allowlist instead of persisting arbitrary exception messages. [fixable]

server/index.ts

The safeguards are directionally sound, but inventory completeness, diagnostic redaction, and rollout configuration need correction before merge.

• 🟡 regressions (L188): Capacity admission is now enabled unconditionally for every configured OpenShell runtime, while the canonical infra/openshell/production.env.example and .env.example do not define the newly required capacity path. Existing deployments following those configurations will start successfully but fail every new sandbox creation—including brokered model discovery—with capacity is unavailable. Add the required setting to deployment configuration and validate it at startup or provide an explicit rollout gate. [fixable]

[dimakis]

🟡 bugs: Inventory scopes are seeded only from lifecycle records, so a provider with no record is never queried. Consequently freshly orphaned sandboxes (for example, a create followed by a crash before provisional registration) and pre-lifecycle sandboxes cannot appear as orphaned; with no records the endpoint incorrectly returns an unavailable empty inventory. Seed collection from sources.accountProviders() as the existing phase-count inventory does. [fixable]

[dimakis]

🟡 unsafe_assumptions: lifecycleError only removes paths and URLs, but provider/CLI errors can contain bearer tokens, grant IDs, or raw response bodies. Such text is returned to clients and durably written to the new audit table, violating the documented sanitized-error boundary. Use stable error codes or an allowlist instead of persisting arbitrary exception messages. [fixable]

[dimakis]

🟡 regressions: Capacity admission is now enabled unconditionally for every configured OpenShell runtime, while the canonical infra/openshell/production.env.example and .env.example do not define the newly required capacity path. Existing deployments following those configurations will start successfully but fail every new sandbox creation—including brokered model discovery—with capacity is unavailable. Add the required setting to deployment configuration and validate it at startup or provide an explicit rollout gate. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (4 warning).

server/openshell-capacity.ts

The safeguards are broadly implemented, but capacity hysteresis and VM collection have operational gaps, provisional inventory can be misreported, and audit failures can obscure completed destructive actions.

• 🟡 bugs (L155): The hysteresis latch exists only in memory and is reset to false whenever the server restarts. If a hard stop was entered below 10% and the server restarts while free space is between the hard and recovery thresholds, the next create is admitted instead of remaining blocked until recovery. Persist the latch or initialize ambiguous post-restart state conservatively. [fixable]
• 🟡 unsafe_assumptions (L108): The documented Podman-VM path option is always passed to the host df executable. A filesystem path inside a Podman VM is not visible to the macOS host, so this supported configuration will remain unavailable and fail every sandbox creation. Collect VM capacity inside the VM or explicitly restrict and validate this setting as a host-visible path. [fixable]

server/openshell-lifecycle-controller.ts

The safeguards are broadly implemented, but capacity hysteresis and VM collection have operational gaps, provisional inventory can be misreported, and audit failures can obscure completed destructive actions.

• 🟡 bugs (L150): Identity-less provisional records are added to seen before configured-provider inventory runs. Even when that provider successfully returns the same physical sandbox, the row is skipped and remains reported as unavailable with a partial scope rather than being reconciled with the physical result. This affects sandboxes whose startup failed after provisional registration. [fixable]

server/app.ts

The safeguards are broadly implemented, but capacity hysteresis and VM collection have operational gaps, provisional inventory can be misreported, and audit failures can obscure completed destructive actions.

• 🟡 bugs (L816): The audit append is inside the same try as the already-completed lifecycle mutation. If audit persistence throws after confirm() stopped/deleted the sandbox (or after consent was persisted), the catch reports the action as failed and attempts another audit write that can also throw, leaving the client with an error despite a successful mutation. Handle audit-write failure separately from the action result or make the persistence boundary explicit. [fixable]
• 🔵 regressions (L723): The inventory exception response omits documented invariant fields such as collectedAt, and its scope omits provider/workspace identity. Consumers cannot use one stable response shape precisely when collection times out or throws. Return the same structured unavailable shape as openShellLifecycleInventory() supplies. [fixable]

[dimakis]

🟡 bugs: The hysteresis latch exists only in memory and is reset to false whenever the server restarts. If a hard stop was entered below 10% and the server restarts while free space is between the hard and recovery thresholds, the next create is admitted instead of remaining blocked until recovery. Persist the latch or initialize ambiguous post-restart state conservatively. [fixable]

[dimakis]

🟡 unsafe_assumptions: The documented Podman-VM path option is always passed to the host df executable. A filesystem path inside a Podman VM is not visible to the macOS host, so this supported configuration will remain unavailable and fail every sandbox creation. Collect VM capacity inside the VM or explicitly restrict and validate this setting as a host-visible path. [fixable]

[dimakis]

🟡 bugs: Identity-less provisional records are added to seen before configured-provider inventory runs. Even when that provider successfully returns the same physical sandbox, the row is skipped and remains reported as unavailable with a partial scope rather than being reconciled with the physical result. This affects sandboxes whose startup failed after provisional registration. [fixable]

[dimakis]

🟡 bugs: The audit append is inside the same try as the already-completed lifecycle mutation. If audit persistence throws after confirm() stopped/deleted the sandbox (or after consent was persisted), the catch reports the action as failed and attempts another audit write that can also throw, leaving the client with an error despite a successful mutation. Handle audit-write failure separately from the action result or make the persistence boundary explicit. [fixable]

[dimakis]

🔵 regressions: The inventory exception response omits documented invariant fields such as collectedAt, and its scope omits provider/workspace identity. Consumers cannot use one stable response shape precisely when collection times out or throws. Return the same structured unavailable shape as openShellLifecycleInventory() supplies. [fixable]

[dimakis]

Centaur Review

Found 4 issue(s) (3 warning).

server/openshell-lifecycle-controller.ts

The safeguards are well covered overall, but inventory is unavailable under the default lifecycle configuration and detached creation can bypass the intended serialization through asynchronous provisioning.

• 🟡 bugs (L121): Inventory is coupled to lifecycle cleanup configuration: configured is initialized only when MITZO_OPENSHELL_LIFECYCLE_ENABLED=1. Since lifecycle cleanup defaults to disabled, an otherwise enabled OpenShell deployment always receives a 503/empty inventory. Initialize the read-only inventory independently whenever the OpenShell runtime is configured. [fixable]
• 🟡 regressions (L175): Inventory uses managerFor(routeRecord), which rejects records whenever the current image or policy digest differs from the persisted lifecycle identity. A normal image/policy deployment therefore marks every sandbox on that route unavailable without querying the gateway, even though those compatibility checks are relevant to mutation, not read-only inventory. Build an inventory-only manager that preserves provider/gateway scope without requiring lifecycle compatibility. [fixable]
• 🔵 bugs (L193): This route-inventory catch swallows aborts, unlike the provider-only catch below. Once the request timeout fires, the loop continues attempting remaining routes and may return a partial 200 if an earlier scope succeeded. Re-throw when signal.aborted so cancellation terminates collection consistently. [fixable]

server/openshell-runtime.ts

The safeguards are well covered overall, but inventory is unavailable under the default lifecycle configuration and detached creation can bypass the intended serialization through asynchronous provisioning.

• 🟡 unsafe_assumptions (L729): The capacity reservation is released as soon as the detached sandbox create command returns, before waitForReady. With MITZO_OPENSHELL_CREATE_DETACHED=1, storage allocation may continue asynchronously, so the next serialized admission can observe stale free space and admit multiple concurrent creations. Retain the reservation until provisioning reaches a stable state, or account for outstanding reservations explicitly. [fixable]

[dimakis]

🟡 bugs: Inventory is coupled to lifecycle cleanup configuration: configured is initialized only when MITZO_OPENSHELL_LIFECYCLE_ENABLED=1. Since lifecycle cleanup defaults to disabled, an otherwise enabled OpenShell deployment always receives a 503/empty inventory. Initialize the read-only inventory independently whenever the OpenShell runtime is configured. [fixable]

[dimakis]

🟡 regressions: Inventory uses managerFor(routeRecord), which rejects records whenever the current image or policy digest differs from the persisted lifecycle identity. A normal image/policy deployment therefore marks every sandbox on that route unavailable without querying the gateway, even though those compatibility checks are relevant to mutation, not read-only inventory. Build an inventory-only manager that preserves provider/gateway scope without requiring lifecycle compatibility. [fixable]

[dimakis]

🔵 bugs: This route-inventory catch swallows aborts, unlike the provider-only catch below. Once the request timeout fires, the loop continues attempting remaining routes and may return a partial 200 if an earlier scope succeeded. Re-throw when signal.aborted so cancellation terminates collection consistently. [fixable]

[dimakis]

🟡 unsafe_assumptions: The capacity reservation is released as soon as the detached sandbox create command returns, before waitForReady. With MITZO_OPENSHELL_CREATE_DETACHED=1, storage allocation may continue asynchronously, so the next serialized admission can observe stale free space and admit multiple concurrent creations. Retain the reservation until provisioning reaches a stable state, or account for outstanding reservations explicitly. [fixable]

[dimakis]

Centaur Review

Found 1 issue(s) (1 critical).

server/openshell-lifecycle-controller.ts

The capacity safeguards are otherwise well covered, but the PR currently has a compile-blocking inventory-controller reference that also breaks the lifecycle-disabled failure path.

• 🔴 regressions (L258): This references the optional mutation controller (configured) while processing read-only inventory. With strict null checks, the PR fails compilation because configured may be undefined. It is also genuinely undefined when lifecycle cleanup is disabled—the mode this PR explicitly supports—so a failed recordless-provider query would throw instead of returning an unavailable scope. Use inventoryConfigured.config.workspace here and add coverage for this failure path with lifecycle disabled. [fixable]

[dimakis]

🔴 regressions: This references the optional mutation controller (configured) while processing read-only inventory. With strict null checks, the PR fails compilation because configured may be undefined. It is also genuinely undefined when lifecycle cleanup is disabled—the mode this PR explicitly supports—so a failed recordless-provider query would throw instead of returning an unavailable scope. Use inventoryConfigured.config.workspace here and add coverage for this failure path with lifecycle disabled. [fixable]

[dimakis]

Centaur Review

Found 2 issue(s) (2 warning).

server/openshell-lifecycle-controller.ts

Two operator-observability gaps remain: historical audits disappear when cleanup is disabled, and some persisted provisional sandboxes are never inventoried.

• 🟡 regressions (L344): The audit endpoint reads only from configured, which is unset whenever lifecycle cleanup is disabled. Because inventory still opens the same persistent lifecycle database, disabling cleanup after previously enabling it causes /api/openshell/lifecycle/audit to return an empty list and hide historical audit entries. Read audit data from inventoryConfigured.store instead. [fixable]
• 🟡 bugs (L159): Providers represented only by identity-less provisional records are excluded from groups. If that provider is no longer returned by the current account-profile callback, inventory never queries it and incorrectly reports the persisted sandbox as unavailable. Include every stored accountProvider in the queried provider scopes; inventory does not require a lifecycle identity. [fixable]

[dimakis]

🟡 regressions: The audit endpoint reads only from configured, which is unset whenever lifecycle cleanup is disabled. Because inventory still opens the same persistent lifecycle database, disabling cleanup after previously enabling it causes /api/openshell/lifecycle/audit to return an empty list and hide historical audit entries. Read audit data from inventoryConfigured.store instead. [fixable]

[dimakis]

🟡 bugs: Providers represented only by identity-less provisional records are excluded from groups. If that provider is no longer returned by the current account-profile callback, inventory never queries it and incorrectly reports the persisted sandbox as unavailable. Include every stored accountProvider in the queried provider scopes; inventory does not require a lifecycle identity. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s) (3 warning).

server/openshell-runtime.ts

The safeguards are broadly structured well, but cancellation can reopen concurrent capacity allocation, inventory can misattribute sandboxes across providers, and unavailable-service action attempts bypass the audit.

• 🟡 bugs (L730): The reservation is released when the caller's signal aborts or waitForReady times out. Because detached creation continues in OpenShell after that signal, another create can be admitted while the first sandbox is still allocating storage, defeating the serialized capacity safeguard. Track provisioning to a terminal state independently of the request signal before releasing the reservation. [fixable]

server/openshell-lifecycle-controller.ts

The safeguards are broadly structured well, but cancellation can reopen concurrent capacity allocation, inventory can misattribute sandboxes across providers, and unavailable-service action attempts bypass the audit.

• 🟡 bugs (L191): Inventory associates a physical sandbox with any record sharing its ID or workspace/name, without requiring record.accountProvider === provider. After provider rebinding or replacement—especially when legacy inventory omits IDs—a sandbox can be reported as verified with another provider's conversation, identity, capabilities, and even stale physical ID. Include provider in both lookup keys and fallback predicates. [fixable]

server/app.ts

The safeguards are broadly structured well, but cancellation can reopen concurrent capacity allocation, inventory can misattribute sandboxes across providers, and unavailable-service action attempts bypass the audit.

• 🟡 regressions (L766): Lifecycle requests made while the service is unavailable return before writing an audit entry; the confirm and consent handlers have the same early return. Moreover, recordOpenShellLifecycleAudit writes only through configured, so the inventory-only store cannot capture these failures when cleanup is disabled. This violates the documented guarantee that all three action attempts are audited and hides attempts during outages or disabled cleanup. [fixable]

[dimakis]

🟡 bugs: The reservation is released when the caller's signal aborts or waitForReady times out. Because detached creation continues in OpenShell after that signal, another create can be admitted while the first sandbox is still allocating storage, defeating the serialized capacity safeguard. Track provisioning to a terminal state independently of the request signal before releasing the reservation. [fixable]

[dimakis]

🟡 bugs: Inventory associates a physical sandbox with any record sharing its ID or workspace/name, without requiring record.accountProvider === provider. After provider rebinding or replacement—especially when legacy inventory omits IDs—a sandbox can be reported as verified with another provider's conversation, identity, capabilities, and even stale physical ID. Include provider in both lookup keys and fallback predicates. [fixable]

[dimakis]

🟡 regressions: Lifecycle requests made while the service is unavailable return before writing an audit entry; the confirm and consent handlers have the same early return. Moreover, recordOpenShellLifecycleAudit writes only through configured, so the inventory-only store cannot capture these failures when cleanup is disabled. This violates the documented guarantee that all three action attempts are audited and hides attempts during outages or disabled cleanup. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s) (3 warning).

server/openshell-runtime.ts

The safeguards are broadly well covered, but ambiguous and cancelled provisioning paths can either bypass serialization or permanently wedge sandbox creation.

• 🟡 regressions (L754): The create command now always receives a fresh, never-aborted signal, even when capacity admission is disabled or createDetached is false. Aborting a session therefore no longer cancels an in-flight CLI create and can delay cancellation until the command's 120-second timeout or leave an unwanted sandbox. Preserve the caller signal outside the detached/capacity-reservation case, or detach the command without blocking the caller. [fixable]
• 🟡 bugs (L756): Any non-conflict error from sandbox create leaves provisioningPending false, so the capacity reservation is released immediately. Timeouts, connection loss, and interrupted responses are ambiguous: the gateway may still be provisioning and consuming storage. Such outcomes need terminal-state observation before releasing the reservation, just like cancellation after a successful response. [fixable]
• 🟡 bugs (L612): The detached terminal watcher never exits if the sandbox is absent before its first successful observation. A create that returns successfully but never becomes visible—or a sandbox deleted before this background loop's first poll—therefore holds the global reservation forever and blocks every subsequent sandbox creation. Carry forward prior observation state or add a bounded absent/terminal policy so this cannot permanently wedge admission. [fixable]

[dimakis]

🟡 regressions: The create command now always receives a fresh, never-aborted signal, even when capacity admission is disabled or createDetached is false. Aborting a session therefore no longer cancels an in-flight CLI create and can delay cancellation until the command's 120-second timeout or leave an unwanted sandbox. Preserve the caller signal outside the detached/capacity-reservation case, or detach the command without blocking the caller. [fixable]

[dimakis]

🟡 bugs: Any non-conflict error from sandbox create leaves provisioningPending false, so the capacity reservation is released immediately. Timeouts, connection loss, and interrupted responses are ambiguous: the gateway may still be provisioning and consuming storage. Such outcomes need terminal-state observation before releasing the reservation, just like cancellation after a successful response. [fixable]

[dimakis]

🟡 bugs: The detached terminal watcher never exits if the sandbox is absent before its first successful observation. A create that returns successfully but never becomes visible—or a sandbox deleted before this background loop's first poll—therefore holds the global reservation forever and blocks every subsequent sandbox creation. Carry forward prior observation state or add a bounded absent/terminal policy so this cannot permanently wedge admission. [fixable]

[dimakis]

Centaur Review

Found 4 issue(s) (3 warning).

server/openshell-runtime.ts

The safeguards are broadly structured well, but inventory completeness/accuracy and an unbounded global capacity reservation need correction before rollout.

• 🟡 bugs (L645): The detached-create reservation can remain held forever when get() continually throws: the catch path ignores absentDeadline and the loop has no cancellation or terminal timeout. Because this reservation is global, one uninspectable provisioning attempt can permanently block every subsequent sandbox create until process restart. [fixable]

server/openshell-lifecycle-controller.ts

The safeguards are broadly structured well, but inventory completeness/accuracy and an unbounded global capacity reservation need correction before rollout.

• 🟡 bugs (L165): Physical inventory is queried only for providers present in lifecycle records or the current account-profile list. A recordless sandbox whose provider profile was removed is therefore omitted entirely instead of appearing as orphaned, even though the underlying workspace list can expose its provider label. [fixable]
• 🟡 bugs (L304): preservationBlockers is hard-coded to report only inventory loss. Verified sandboxes with active sessions, queued work, task-board ownership, unavailable checkpoints, or other lifecycle blockers are returned with an empty blocker list, making the operator inventory disagree with the subsequent preview safeguard. [fixable]

server/app.ts

The safeguards are broadly structured well, but inventory completeness/accuracy and an unbounded global capacity reservation need correction before rollout.

• 🔵 regressions (L743): The capacity endpoint's unconfigured and exception responses omit collectedAt and per-metric availability, while successful snapshots and the documented API are timestamped and explicitly represent unavailable metrics. Clients cannot distinguish a fresh unavailable result from an old or malformed response. [fixable]

[dimakis]

🟡 bugs: The detached-create reservation can remain held forever when get() continually throws: the catch path ignores absentDeadline and the loop has no cancellation or terminal timeout. Because this reservation is global, one uninspectable provisioning attempt can permanently block every subsequent sandbox create until process restart. [fixable]

[dimakis]

🟡 bugs: Physical inventory is queried only for providers present in lifecycle records or the current account-profile list. A recordless sandbox whose provider profile was removed is therefore omitted entirely instead of appearing as orphaned, even though the underlying workspace list can expose its provider label. [fixable]

[dimakis]

🟡 bugs: preservationBlockers is hard-coded to report only inventory loss. Verified sandboxes with active sessions, queued work, task-board ownership, unavailable checkpoints, or other lifecycle blockers are returned with an empty blocker list, making the operator inventory disagree with the subsequent preview safeguard. [fixable]

[dimakis]

🔵 regressions: The capacity endpoint's unconfigured and exception responses omit collectedAt and per-metric availability, while successful snapshots and the documented API are timestamped and explicitly represent unavailable metrics. Clients cannot distinguish a fresh unavailable result from an old or malformed response. [fixable]

[dimakis]

Centaur Review

Found 4 issue(s) (4 warning).

server/openshell-lifecycle-controller.ts

The safeguards add useful coverage, but ownership verification and reservation lifecycle edge cases can misreport sandboxes, overcommit capacity, or block all future creates.

• 🟡 unsafe_assumptions (L217): A physical sandbox is classified as verified using only provider plus physical ID. Its persisted sandbox name, workspace, and mitzo.conversation ownership label are not checked, so a stale/reassigned ID or modified label can be presented as the record's verified, action-capable sandbox instead of an ambiguous orphan. Validate all ownership identity fields before assigning verified. [fixable]

server/openshell-runtime.ts

The safeguards add useful coverage, but ownership verification and reservation lifecycle edge cases can misreport sandboxes, overcommit capacity, or block all future creates.

• 🟡 bugs (L659): The background observer releases the global create reservation after the readiness deadline when inventory remains unavailable, even though a successful detached create may still be provisioning. A subsequent admission can then sample before that allocation is reflected and issue another create, defeating the serialized fail-closed safeguard. Unobservable provisioning needs a durable reservation/fence or another authoritative terminal check. [fixable]
• 🟡 bugs (L652): Once a sandbox is observed in Creating, Pending, or Starting, the deadline is no longer consulted on successful polls. A sandbox stuck indefinitely in any of those phases therefore holds the single global capacity reservation forever and blocks every future physical sandbox create. [fixable]

server/openshell-capacity.ts

The safeguards add useful coverage, but ownership verification and reservation lifecycle edge cases can misreport sandboxes, overcommit capacity, or block all future creates.

• 🟡 regressions (L211): Waiting for the preceding capacity reservation does not observe the caller's AbortSignal. A cancelled session queued behind a slow or stuck create remains pending until that create releases, contrary to the cancellation behavior used elsewhere in runtime provisioning. Race the queue wait with the signal and ensure the queued gate is released safely on cancellation. [fixable]

[dimakis]

🟡 unsafe_assumptions: A physical sandbox is classified as verified using only provider plus physical ID. Its persisted sandbox name, workspace, and mitzo.conversation ownership label are not checked, so a stale/reassigned ID or modified label can be presented as the record's verified, action-capable sandbox instead of an ambiguous orphan. Validate all ownership identity fields before assigning verified. [fixable]

[dimakis]

🟡 bugs: The background observer releases the global create reservation after the readiness deadline when inventory remains unavailable, even though a successful detached create may still be provisioning. A subsequent admission can then sample before that allocation is reflected and issue another create, defeating the serialized fail-closed safeguard. Unobservable provisioning needs a durable reservation/fence or another authoritative terminal check. [fixable]

[dimakis]

🟡 bugs: Once a sandbox is observed in Creating, Pending, or Starting, the deadline is no longer consulted on successful polls. A sandbox stuck indefinitely in any of those phases therefore holds the single global capacity reservation forever and blocks every future physical sandbox create. [fixable]

[dimakis]

🟡 regressions: Waiting for the preceding capacity reservation does not observe the caller's AbortSignal. A cancelled session queued behind a slow or stuck create remains pending until that create releases, contrary to the cancellation behavior used elsewhere in runtime provisioning. Race the queue wait with the signal and ensure the queued gate is released safely on cancellation. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s) (3 warning).

server/openshell-runtime.ts

The safeguards are broadly structured well, but detached-create failures can permanently halt admission and inventory can misclassify or silently omit incomplete collection states.

• 🟡 bugs (L648): A detached create that fails before creating a sandbox leaves observed false. Successful get calls then return absence until the deadline, but this path installs an infinite provisioning fence instead of treating sustained authoritative absence as terminal. Consequently an ordinary create failure (invalid image/policy, rejected request, etc.) can block every later sandbox creation until the server restarts. Track whether inventory was successfully queried and release after the bounded window when the sandbox remained absent; reserve the permanent fence for unavailable inventory or an observed transitional sandbox. [fixable]

server/openshell-lifecycle-controller.ts

The safeguards are broadly structured well, but detached-create failures can permanently halt admission and inventory can misclassify or silently omit incomplete collection states.

• 🟡 bugs (L340): Sandbox.workspace is optional, and managedInventory() explicitly accepts rows where it is omitted, but ownership matching requires it to equal the persisted workspace. Gateways omitting this optional field therefore turn valid lifecycle records into anonymous orphaned rows (and the earlier seen insertion suppresses the corresponding missing record). Treat an absent workspace as the configured/query workspace, as the inventory filter already does. [fixable]
• 🟡 unsafe_assumptions (L199): Workspace-wide provider discovery failure is reported only when there are no known provider groups/scopes. If any tracked or configured provider exists and its scoped query succeeds, the response claims partial: false even though recordless sandboxes belonging to removed providers could not be discovered. Always represent a failed inventoryAll discovery as an unavailable scope so collection health does not falsely appear complete. [fixable]

[dimakis]

🟡 bugs: A detached create that fails before creating a sandbox leaves observed false. Successful get calls then return absence until the deadline, but this path installs an infinite provisioning fence instead of treating sustained authoritative absence as terminal. Consequently an ordinary create failure (invalid image/policy, rejected request, etc.) can block every later sandbox creation until the server restarts. Track whether inventory was successfully queried and release after the bounded window when the sandbox remained absent; reserve the permanent fence for unavailable inventory or an observed transitional sandbox. [fixable]

[dimakis]

🟡 bugs: Sandbox.workspace is optional, and managedInventory() explicitly accepts rows where it is omitted, but ownership matching requires it to equal the persisted workspace. Gateways omitting this optional field therefore turn valid lifecycle records into anonymous orphaned rows (and the earlier seen insertion suppresses the corresponding missing record). Treat an absent workspace as the configured/query workspace, as the inventory filter already does. [fixable]

[dimakis]

🟡 unsafe_assumptions: Workspace-wide provider discovery failure is reported only when there are no known provider groups/scopes. If any tracked or configured provider exists and its scoped query succeeds, the response claims partial: false even though recordless sandboxes belonging to removed providers could not be discovered. Always represent a failed inventoryAll discovery as an unavailable scope so collection health does not falsely appear complete. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s) (2 warning).

server/openshell-capacity.ts

The safeguards are broadly well tested, but cancellation can incorrectly trip admission, an empty healthy inventory returns 503, and fallback Podman units are miscalculated.

• 🟡 bugs (L110): collect() uses Promise.allSettled() and converts aborted child commands into unavailable metrics instead of propagating the abort. During admission, a cancelled request therefore throws OpenShellCapacityError and sets the shared hard-stop latch, potentially blocking later creates until the recovery threshold is reached. Re-throw when signal.aborted before interpreting the settled results. [fixable]
• 🔵 bugs (L75): The byte parser accepts both SI units such as MB and IEC units such as MiB, but multiplies both by powers of 1024. When Podman's raw fields are absent and the display fields are used as fallback, SI-formatted usage and reclaimable values are overstated. Preserve the optional i and use powers of 1000 for SI units. [fixable]

server/openshell-lifecycle-controller.ts

The safeguards are broadly well tested, but cancellation can incorrectly trip admission, an empty healthy inventory returns 503, and fallback Podman units are miscalculated.

• 🟡 bugs (L315): A successful empty workspace is reported as unavailable. If inventoryAll() succeeds but finds no sandboxes, there are no lifecycle records, and accountProviders() is empty, no scope is added; available becomes false and the endpoint returns 503. Record a successful configured/workspace scope so an authoritative empty inventory returns 200 with sandboxes: []. [fixable]

[dimakis]

🟡 bugs: collect() uses Promise.allSettled() and converts aborted child commands into unavailable metrics instead of propagating the abort. During admission, a cancelled request therefore throws OpenShellCapacityError and sets the shared hard-stop latch, potentially blocking later creates until the recovery threshold is reached. Re-throw when signal.aborted before interpreting the settled results. [fixable]

[dimakis]

🔵 bugs: The byte parser accepts both SI units such as MB and IEC units such as MiB, but multiplies both by powers of 1024. When Podman's raw fields are absent and the display fields are used as fallback, SI-formatted usage and reclaimable values are overstated. Preserve the optional i and use powers of 1000 for SI units. [fixable]

[dimakis]

🟡 bugs: A successful empty workspace is reported as unavailable. If inventoryAll() succeeds but finds no sandboxes, there are no lifecycle records, and accountProviders() is empty, no scope is added; available becomes false and the endpoint returns 503. Record a successful configured/workspace scope so an authoritative empty inventory returns 200 with sandboxes: []. [fixable]

[dimakis]

Centaur Review

Found 3 issue(s) (3 warning).

server/openshell-runtime.ts

The safeguards are broadly well covered, but detached-create fencing and inventory reconciliation still have fail-open or misleading edge cases.

• 🟡 bugs (L671): A detached create is considered definitively absent when the deadline expires after merely one successful get returning no sandbox (inventoryQueried is only a boolean). A slow gateway/create can remain invisible for the entire readiness window and materialize later, but this path releases the reservation without installing a provisioning fence, allowing another create despite unresolved capacity consumption. Require sustained/repeated absence after the create has settled, or retain a fence until terminal state is proven. [fixable]

server/openshell-lifecycle-controller.ts

The safeguards are broadly well covered, but detached-create fencing and inventory reconciliation still have fail-open or misleading edge cases.

• 🟡 bugs (L208): Provider groups are keyed only by provider, and managerForProvider() always queries the currently configured workspace, but the resulting scope is labeled with routeRecord.workspace. After a workspace configuration change, retained records from the old workspace are therefore reported under an apparently available old-workspace scope even though that workspace was never queried, and their sandboxes are misleadingly classified as missing. Group/query by provider plus workspace (and relevant gateway identity), or report the actual queried scope and mark inaccessible historical scopes unavailable. [fixable]
• 🟡 bugs (L215): The physical identity key is added to seen before ownership validation. If a sandbox reuses a persisted physical ID/name but has a mismatched conversation label, the orphan row consumes the key and the final record pass suppresses the corresponding missing lifecycle row. This hides the affected conversation, checkpoint, consent, and failure state from operator inventory. Track physical observations separately from matched lifecycle records so the orphaned physical sandbox and unmatched persisted record are both surfaced. [fixable]

[dimakis]

🟡 bugs: A detached create is considered definitively absent when the deadline expires after merely one successful get returning no sandbox (inventoryQueried is only a boolean). A slow gateway/create can remain invisible for the entire readiness window and materialize later, but this path releases the reservation without installing a provisioning fence, allowing another create despite unresolved capacity consumption. Require sustained/repeated absence after the create has settled, or retain a fence until terminal state is proven. [fixable]

[dimakis]

🟡 bugs: Provider groups are keyed only by provider, and managerForProvider() always queries the currently configured workspace, but the resulting scope is labeled with routeRecord.workspace. After a workspace configuration change, retained records from the old workspace are therefore reported under an apparently available old-workspace scope even though that workspace was never queried, and their sandboxes are misleadingly classified as missing. Group/query by provider plus workspace (and relevant gateway identity), or report the actual queried scope and mark inaccessible historical scopes unavailable. [fixable]

[dimakis]

🟡 bugs: The physical identity key is added to seen before ownership validation. If a sandbox reuses a persisted physical ID/name but has a mismatched conversation label, the orphan row consumes the key and the final record pass suppresses the corresponding missing lifecycle row. This hides the affected conversation, checkpoint, consent, and failure state from operator inventory. Track physical observations separately from matched lifecycle records so the orphaned physical sandbox and unmatched persisted record are both surfaced. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (5 warning).

server/openshell-runtime.ts

The core safeguards are substantial, but several fail-closed and observability gaps remain, including an unguarded probe-sandbox creation path.

• 🟡 bugs (L761): Capacity admission is enforced only in OpenShellRuntimeManager.ensure(), but OpenShellConnectionGateway.probe() has another detached sandbox create path. Connection provisioning can therefore create a physical probe sandbox during a capacity hard stop, contradicting the safeguard's all-new-sandboxes contract. Route probe creation through the same reservation/fencing mechanism and add an integration test. [fixable]
• 🟡 bugs (L675): An unresolved provisioning attempt starts an infinite observer with a never-aborted signal and the normal 250 ms readiness polling interval. A persistently unavailable gateway can therefore spawn OpenShell CLI processes indefinitely (and the observer is not stopped during graceful shutdown). Use a cancellable, centrally owned observer with bounded/exponential polling while retaining the fail-closed fence. [fixable]

server/openshell-lifecycle-controller.ts

The core safeguards are substantial, but several fail-closed and observability gaps remain, including an unguarded probe-sandbox creation path.

• 🟡 bugs (L262): If the shared request signal expires while collecting a later provider, this branch rethrows and app.ts replaces the entire accumulated inventory with an empty generic 503 response. Successful scopes and sandbox rows collected earlier are lost, contrary to the partial-inventory contract. Preserve completed results and mark the interrupted/current and remaining scopes unavailable. [fixable]
• 🟡 bugs (L334): Records skipped at lines 171-190 because their persisted gateway/workspace no longer matches the configured runtime fall through here and are labeled missing. Their physical scope was never queried, so absence was not established; these rows must be unavailable rather than missing to avoid falsely reporting that the sandbox is gone. [fixable]

server/openshell-capacity.ts

The core safeguards are substantial, but several fail-closed and observability gaps remain, including an unguarded probe-sandbox creation path.

• 🟡 bugs (L188): The status calculation ignores provisioningFences. After an ambiguous detached create times out, reserveNewSandbox() rejects every new create as unresolved while /api/openshell/capacity can still report state: normal. Include the fence in the reported admission state or expose an explicit unresolved-provisioning field. [fixable]

[dimakis]

🟡 bugs: Capacity admission is enforced only in OpenShellRuntimeManager.ensure(), but OpenShellConnectionGateway.probe() has another detached sandbox create path. Connection provisioning can therefore create a physical probe sandbox during a capacity hard stop, contradicting the safeguard's all-new-sandboxes contract. Route probe creation through the same reservation/fencing mechanism and add an integration test. [fixable]

[dimakis]

🟡 bugs: An unresolved provisioning attempt starts an infinite observer with a never-aborted signal and the normal 250 ms readiness polling interval. A persistently unavailable gateway can therefore spawn OpenShell CLI processes indefinitely (and the observer is not stopped during graceful shutdown). Use a cancellable, centrally owned observer with bounded/exponential polling while retaining the fail-closed fence. [fixable]

[dimakis]

🟡 bugs: If the shared request signal expires while collecting a later provider, this branch rethrows and app.ts replaces the entire accumulated inventory with an empty generic 503 response. Successful scopes and sandbox rows collected earlier are lost, contrary to the partial-inventory contract. Preserve completed results and mark the interrupted/current and remaining scopes unavailable. [fixable]

[dimakis]

🟡 bugs: Records skipped at lines 171-190 because their persisted gateway/workspace no longer matches the configured runtime fall through here and are labeled missing. Their physical scope was never queried, so absence was not established; these rows must be unavailable rather than missing to avoid falsely reporting that the sandbox is gone. [fixable]

[dimakis]

🟡 bugs: The status calculation ignores provisioningFences. After an ambiguous detached create times out, reserveNewSandbox() rejects every new create as unresolved while /api/openshell/capacity can still report state: normal. Include the fence in the reported admission state or expose an explicit unresolved-provisioning field. [fixable]

[dimakis]

Centaur Review

Found 5 issue(s) (3 warning).

server/openshell-runtime.ts

The safeguards are broadly structured well, but create admission has two fail-closed lifecycle gaps plus configuration and inventory-reporting defects.

• 🟡 regressions (L761): The sandbox absence check occurs before waiting for serialized capacity admission. If another concurrent ensure creates the same sandbox while this caller waits, a later hard-stop sample rejects this caller instead of reattaching to the now-existing sandbox, contradicting the guarantee that hard stops block only new physical creation. Recheck ownership/existence inside the serialized create section before applying admission. [fixable]
• 🟡 unsafe_assumptions (L671): Repeated not found responses are treated as proof that detached provisioning is terminal, even after the create command succeeded. A delayed/eventually-consistent gateway can hide an accepted allocation past the readiness windows, causing the reservation to be released without installing a fence while storage is still being allocated. Successful creates need to remain fenced until positive terminal evidence is observed. [fixable]

server/openshell-capacity.ts

The safeguards are broadly structured well, but create admission has two fail-closed lifecycle gaps plus configuration and inventory-reporting defects.

• 🟡 unsafe_assumptions (L53): Capacity rollout accepts any readable relative path, despite the configuration contract requiring an authoritative host storage path. A value such as . silently monitors the server working directory's filesystem and can admit creates while the actual Podman storage filesystem is full. Require an absolute path before enabling admission. [fixable]

server/openshell-lifecycle-controller.ts

The safeguards are broadly structured well, but create admission has two fail-closed lifecycle gaps plus configuration and inventory-reporting defects.

• 🔵 bugs (L112): Phase-count deduplication keys only on sandbox ID/name across all provider-scoped inventories. The PR's inventory model permits different providers to return the same physical ID, so those sandboxes are collapsed and lifecycle observability undercounts phases. Include the provider/workspace in the deduplication key. [fixable]
• 🔵 bugs (L220): If both workspace discovery and the configured-provider callback fail, each catch appends the same configured/workspace unavailable scope, producing duplicate scope rows. Deduplicate scopes by provider/workspace or consolidate these failures before returning the operator inventory. [fixable]