Skip to content

Start the API before enabling workers on an existing installation - #3253

Open
freeqaz-openai wants to merge 6 commits into
mainfrom
freeqaz/named-workspace-runtime-and-startup-gate
Open

freeqaz-openai wants to merge 6 commits into
mainfrom
freeqaz/named-workspace-runtime-and-startup-gate

Conversation

@freeqaz-openai

@freeqaz-openai freeqaz-openai commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

An operator can start an API against an initialized installation with the worker at zero, verify the Installation ID, logical Namespace ID and Kubernetes namespace UID, then enable one worker. This implements the runtime portion of the submitted installation-profile amendment. It does not bootstrap or migrate the database, activate Git credentials, or move traffic.

A named worker now processes queue and cleanup work only for its configured namespace. A PostgreSQL regression covers both startup modes: the named worker completes its own work without changing queued or expired claims in a neighboring namespace, while an unbound worker continues to process both.

Human security and architecture review is required before landing. Namespace discovery uses an exact-name read. Within that namespace, the API has tenant and configuration permissions, including Secrets; the worker receives its corresponding bindings only when enabled. Kubernetes RBAC follows the namespace name, and the UID check and subsequent write are not atomic.

The optional gateway-readiness gate checks Health for availability and GetCurrentUser for a gateway-accepted caller. Our previous fixture incorrectly treated Health as authenticated; stock OpenShell v0.1.5 distinguishes the two. The gate requires a nonempty subject and a supported identity provider matching the selected transport. It does not pin a human subject or establish authorization for future work. With a refused credential, the corrected worker leaves queued and previously claimed work untouched; an accepted replacement token permits recovery under the original deadlines. Independent startup remains the default.

The evidence below separates the stock gateway comparison, the PostgreSQL worker test with a controlled gateway, and real Kubernetes API permission checks. These are local, synthetic checks; no staging deployment or qualification is claimed. Hosted CI has not yet run on this successor head.

OpenShell v0.1.5: observed calls and old versus corrected startup

We tested the published x86-64 openshell-gateway 0.1.5 executable (SHA-256 c8fa78fd49f245978f64d038207ce0205d98fcdd48bf115bae8a0b9f4f4d5003) over verified TLS. OIDC signing and compute discovery were local fixtures; the gateway and its authentication router were the release binary. No Sandbox was started, and we did not retain a token or caller identity.

Stock gateway call             Synthetic caller             Observed gRPC result
Health                         incorrect, nonempty bearer   OK, healthy
GetCurrentUser                 same incorrect bearer        UNAUTHENTICATED
GetCurrentUser                 valid OIDC bearer            OK, expected subject and provider
GetCurrentUser                 incorrect after success      UNAUTHENTICATED
GetCurrentUser                 expired OIDC bearer          UNAUTHENTICATED

Actual OCE startup check       Incorrect -> valid -> incorrect bearer, same gateway
Before                         ready -> ready -> ready
After                          identity-unavailable -> ready -> identity-unavailable

This verifies a gateway-accepted caller at startup, not authorization for future Workspace operations or immediate revocation of an already valid token. The live gateway comparison used OIDC; the other provider checks are not claims of deployed mTLS or Cloudflare coverage.

Kubernetes v1.35: redacted API, worker, revocation and replacement audit

Actual Kubernetes API server audit, stage ResponseComplete; synthetic namespace and service-account names shortened. Each pair is HTTP status / audit-ID prefix from the receiver. Same API and worker credentials throughout. Neighbor and replacement raw creates are server dry-runs; other creates are real fixture ConfigMaps.

State               Caller  Receiver operation and result (status/audit ID)
zero workers       api     GET namespace 200/b6911c30; create ConfigMap 201/25cf97bc
zero workers       worker  GET namespace 200/c63eff86; create ConfigMap 403/5ee4ad50
off target         api     create ConfigMap 403/1f24a795
off target         worker  create ConfigMap 403/81364ec8
worker enabled     worker  GET namespace 200/1e812bc7; create ConfigMap 201/55393c5c
bindings revoked   worker  GET namespace 200/1cab79c6; create ConfigMap 403/8a915d25
bindings revoked   api     GET namespace 200/3e971567; create ConfigMap 201/39be5cac
namespace replaced api     create ConfigMap 403/af067977
API regranted      api     create ConfigMap 201/f94f4ed6
OCE holds old UID  api     GET namespace 200/821543c2; OCE OwnershipFailure; no create in invocation

Metadata stream SHA-256: 8eb2d5f94108535e22803c8510368d95807c728df58688f1c43bc619f93840da. No workload was created. This is a finite observation, not an atomic UID condition on a future write. The API retains namespace-wide tenant/configuration permissions, including Secrets.

@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added the clawsweeper:intake:feature Preliminary submission classification; full source review pending. label Oct 23, 2026
@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed clawsweeper:intake:feature Preliminary submission classification; full source review pending. labels Oct 23, 2026
@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: blocked before merge.

What this changes

This PR adds attachment to an initialized installation, staged worker permissions, optional authenticated startup gating, and namespace-scoped worker processing.

Example: An installation contains logical Namespaces ns_alpha and ns_beta, and its named worker is bound to ns_alpha.

  • Before: The worker could recover expired claims and sweep Harness exposures in both Namespaces.
  • After: The named worker processes ns_alpha while leaving ns_beta's work and maintenance untouched.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) Useful, coherent implementation with scoped real gateway and Kubernetes evidence, meaningful PostgreSQL coverage, and no discrete correctness finding; human approval remains enforced.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (logs): Supplied stock OpenShell output exercises the production startup check with incorrect, valid, and subsequently refused credentials; Kubernetes receiver audits show staged write permissions, neighboring-Namespace denial, revocation, and old-UID refusal before creation. These scoped results remain applicable to unchanged paths; the successor's namespace filtering is additionally supported by inspected PostgreSQL regression code, without a new execution claim or production qualification.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Feature · Worth it: Yes · Fix scope: Complete
User problem: Operators need to attach an API to a prepared installation before granting and enabling worker reconciliation.
Reason: The maintainer-authored direction addresses a concrete staged-deployment need and clearly limits this PR to runtime attachment.

Merge readiness

⛔ Blocked before merge - 1 item remains

Keep this PR open for the explicitly required human security and architecture review; no actionable introduced bug was found in the pinned head.

Priority: P2
Reviewed head: d37f3988e58ef3654f87e99a6904b7cd9eb30abb

Before merge

  • Await maintainer action - Complete the required human security and architecture review of d37f398 before landing.

Findings

None.

Tests

  • Missing end-to-end proof: The successor namespace regression has no separately supplied execution result; its Harness cleanup assertion proves dispatch selection rather than a live withdrawal.
Agent review details

How this fits together

Production composition reads the operator's installation binding, verifies PostgreSQL and Kubernetes identities, and configures the API and worker; the worker consumes durable work through the selected Drivers.

flowchart TB
  A[Operator binding] --> B[Production composition]
  B --> C[PostgreSQL identity checks]
  B --> D[Kubernetes UID checks]
  C --> E[API readiness]
  D --> E
  B --> F[Optional gateway startup gate]
  F --> G[Namespace scoped worker]
  G --> H[Queue and Harness maintenance]
Loading

Technical review

Best possible solution:

Retain the opt-in runtime attachment and namespace-scoped worker implementation, subject to the existing human security and architecture approval.

Do we have a high-confidence way to reproduce the issue?

This is a new capability; source comparison confirms that current main provides the diagnostic binding rather than the requested Git-disabled runtime attachment.

Is this the best way to solve the issue?

The implementation reuses existing binding, queue, and Driver owners, and the new filters address scheduling and recovery together without adding a competing worker path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against eda1f15542cc.

Provenance checked

  • OpenShell Backend membership changes intended behavior with a stated reason (d4f544c: The original Backend shared one gateway connection between Sandbox and Credential Gateway integration.)
  • Harness exposure maintenance keeps the original intent (2946c0e: Sweep native revisions without preparation passes so retained exposures are withdrawn; the new binding narrows enumeration while preserving that cleanup.)
  • Preprovisioned installation binding changes intended behavior with a stated reason (3115679: The existing binding supported a closed credential-delivery diagnostic profile; this PR explicitly adds a separate Git-disabled runtime profile under the submitted amendment.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

  • Pinned introduction and re-review scope: Reviewed the merge-base-to-head introduction and the successor change from 993ba46 to d37f398; the latter narrows queue operations and Harness maintenance without changing the authenticated startup gate. (apps/controller/src/worker.ts:1172, d37f3988e58e)
  • Queue scope reaches recovery and scheduling: The configured Namespace reaches claim selection, both stale/exhausted recovery queries, pending counts, claimable-work checks, and startup age reads; omitted binding retains cross-Namespace scheduling. (packages/occ/src/state/postgres-work-queue.ts:1743, d37f3988e58e)
  • Maintenance scope and regression coverage: The exposure sweep selects the bound Namespace before enumerating Agents and revisions. The PostgreSQL test checks unchanged neighboring queued, expired, and exhausted rows under both startup policies, plus an unbound-worker control; exposure dispatch is observed through a mocked Compute method. (tests/integration/named-workspace-runtime.test.ts:698, d37f3988e58e)
  • Supplied behavior and authority evidence: The full PR body supplies stock-gateway before/after output and Kubernetes receiver audit results for allowed writes, neighboring-Namespace denial, zero-worker denial, binding revocation, and replacement-UID refusal; it expressly retains human security and architecture review. (d37f3988e58e)
  • Submitted contract proposal: RFC 0018 amendment: first install with a named Namespace and gated reconciliation #2834 proposes exact Namespace attachment and startup gating before implementation; this PR explicitly delivers the runtime portion rather than bootstrap or worker-hosted Git transport.
  • Authoritative dependency protocol: The changed client and wire fixture consume this protocol, establishing the dependency-policy signal. The pinned upstream contract declares Health unauthenticated and GetCurrentUser authenticated, with subject field 1 and identity_provider field 5; its root policy was also read. (proto/openshell.proto:24, bdb956f2cd65)

Labels

Label changes:

No label changes.

Label justifications:

  • P2: This is a bounded operator deployment capability with no demonstrated emergency or urgent regression.
  • merge-risk: 🚨 security-boundary: The profile changes tenant RoleBindings and worker scope; the recorded design retains namespace-wide API Secret permissions and a non-atomic UID-check/write boundary.
  • merge-risk: 🚨 compatibility: The opt-in profile changes installation configuration and startup behavior, and permits sandbox-only OpenShell Backends while preserving existing selections.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
  • status: ⏳ waiting on maintainer: Only explicit maintainer approvals or decisions remain; merge is still blocked.
  • proof: sufficient: Contributor real behavior proof is sufficient.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

History

Review history (3 earlier review cycles)
  • reviewed 2026-10-23T01:17:24.609Z sha 70a74c6 :: needs real behavior proof before merge. :: [P2] Preserve case-insensitive HTTPS recognition in the startup gate
  • reviewed 2026-10-23T15:03:27.295Z sha 5c7ffab :: needs real behavior proof before merge. :: [P2] Authenticate the gateway identity before opening the startup gate
  • reviewed 2026-10-23T15:38:54.003Z sha 993ba46 :: blocked before merge. :: none

Reviewed October 26, 2026, 3:50 PM ET / 19:50 UTC (Revision 4).

@freeqaz-openai freeqaz-openai changed the title feat: attach named workspaces before enabling worker claims Start the API before enabling workers on an existing installation Oct 23, 2026
@freeqaz-openai

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

Please review the updated draft at 5c7ffab23c3a7ebbc96e81b37722ad8e634ee1da. We addressed the earlier review without widening this profile’s permissions.

  • Readiness: The eleventh request from one direct peer returned 429 before additional PostgreSQL or Kubernetes work. A second peer still obtained fresh readiness. This is a per-process, per-peer limit.
  • Gateway compatibility: The same PostgreSQL and authenticated TLS Health regression failed on the old case-sensitive check and passed here, including HTTPS, hTtPs, and the existing clear refusal for a bare IPv6 address. TLS and authentication remain required.
  • Effective permissions: Against a real Kubernetes 1.35 API server, the API’s ConfigMap create succeeded with the worker at zero; the worker and both neighboring-namespace controls were denied. The same worker token was allowed after its bindings were added and denied after they were removed. On namespace replacement, a direct dry-run create was allowed after regrant; OCE with the old UID read the namespace and sent no create. We checked the API server’s audit records. RBAC is name-based; checking the UID and writing are not atomic.

This was local qualification with no workloads started, not an EKS rollout. Please keep the security and architecture decisions with the human reviewer.

@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:feature Preliminary submission classification; full source review pending. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. clawsweeper:intake:feature Preliminary submission classification; full source review pending. labels Oct 23, 2026
@freeqaz-openai

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

Fixed the public-Health mistake at 993ba4674bed65cf6a31410760cea1ee1d53189d. Thank you for catching it. The PR body now includes the actual stock-gateway comparison and redacted API-server audit events requested in the previous review.

  • Against the released OpenShell v0.1.5 binary over verified TLS, the same wrong bearer got healthy from public Health and UNAUTHENTICATED from GetCurrentUser. On that same gateway, actual OCE with an incorrect, valid, then incorrect synthetic bearer returned ready / ready / ready before and identity-unavailable / ready / identity-unavailable after.
  • With real PostgreSQL and the final contract-faithful wire fixture, the same test failed on the old unexpected ready and passed here, zero skips. The worker got two rejected identity reads while both previously claimed and fresh queued rows stayed unchanged; replacing the token file then let normal recovery proceed.
  • The namespace-wide permission decision, non-atomic UID check and human security/architecture review are unchanged. Nothing was deployed by this update.

@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:feature Preliminary submission classification; full source review pending. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: ⏳ waiting on maintainer Only explicit maintainer approvals or decisions remain; merge is still blocked. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. clawsweeper:intake:feature Preliminary submission classification; full source review pending. labels Oct 23, 2026
Comment thread apps/controller/src/composition/named-workspace.ts
Comment thread apps/controller/src/worker/startup-gate.ts
@freeqaz-openai

Copy link
Copy Markdown
Collaborator Author

@clawsweeper re-review

Please review d37f398. The named worker now limits queue processing and Harness maintenance to its configured namespace. The PostgreSQL regression covers both startup policies and a worker without a namespace binding.

@clawsweeper

clawsweeper Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
Exact review queued.

Re-review progress:

@clawsweeper clawsweeper Bot added clawsweeper:intake:feature Preliminary submission classification; full source review pending. and removed clawsweeper:intake:feature Preliminary submission classification; full source review pending. labels Oct 26, 2026
@freeqaz

freeqaz commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Free's clanker here (an AI agent working on Free's behalf).

I checked this PR for landing on head d37f3988 and am leaving it unmerged. CI Required and First Agent Smoke pass on this head, and GitHub reports it as mergeable. The blockers are scope and review gates, not CI:

  1. Human security and architecture review is still required. The PR body asks for it, and the latest ClawSweeper verdict on d37f3988 lists it as the one remaining item. Nobody has submitted a review (reviewDecision is REVIEW_REQUIRED). The change touches tenant isolation and Kubernetes RBAC (deploy/helm/openclaw-enterprise/templates/rbac.yaml, serviceaccounts.yaml, networkpolicies.yaml), where the API keeps namespace-wide Secret access, and the UID check before a write is not atomic. An automated lane should not decide these.
  2. The design it implements is not accepted yet. The PR implements the runtime part of the RFC 0018 amendment in RFC 0018 amendment: first install with a named Namespace and gated reconciliation #2834, which is still open. It also adds a new capability: named-namespace attachment, the gateway-readiness startup gate, and new Helm values and settings (docs/reference/settings/named-workspace.md). Under the current quality-only scope, features land after the RFC is accepted and a human approves.

To unblock: accept #2834, get a maintainer approval for the RBAC and tenant-isolation design, then merge main and let CI run again. The branch is 450 commits behind main, so a main merge plus fresh CI is needed before landing anyway. I did not push anything to this branch.

This branch has not been deployed

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

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: ⏳ waiting on maintainer Only explicit maintainer approvals or decisions remain; merge is still blocked.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants