[jhjaggars]

Problem

Two development-infrastructure gaps, found while driving the Compose control plane with Kubernetes Compute under rootful Podman:

• The dev launcher selected podman-compose whenever Podman was the engine, but podman-compose does not support --project-directory, which the launcher's Compose command passes — so occ dev up could not run against Podman. occ dev down resolved only podman-compose, so a host with only docker-compose could start the stack but not stop it.
• When startup failed, the launcher rolled back its cluster, Compose stack, and state directory, removing the evidence needed to diagnose the failure.

This is the first of four independently mergeable slices split from a larger OpenShell integration branch; it deliberately carries no OpenShell semantics.

Change

• fix(dev): prefer docker-compose for Compose-mode Podman — the Kubernetes-profile Compose command that occ dev up and occ dev down drive prefers docker-compose (which reaches Podman through the DOCKER_HOST endpoint startup pins) over podman-compose, keeping podman-compose for hosts without it. One shared helper resolves the provider for both commands. Docker Compute keeps standalone podman-compose on both ends: scripts/dev-up pins it to start, and occ dev down selects the same provider to stop, so a provider mismatch cannot leave a started stack running. first-agent.mjs records the same preference when it replays saved state. The flow, driver reference, and requirements documentation state the split, including the rootful Podman API socket the DOCKER_HOST route needs.
• feat(dev): keep a failed development stack for inspection — OCC_DEVELOPMENT_KEEP_ON_FAILURE, set to any nonempty value, keeps the failed stack, state directory, and any copied service key, and prints a cleanup hint naming occ dev down. Both the Kubernetes-only and Compose control-plane profiles honor it. Documented in the development settings reference and the environment variables cheat sheet.

Dropped from the earlier revision after review: the 0644 routing-file mode changes (they broke assignComposeFileOwner's private-file check), the worker and Kubernetes Driver log-field changes (they bypassed the bounded diagnostic paths and failed the postgres-worker-agent-revision integration test), and the Compose postgres healthcheck change (main's TCP pg_isready check with start_period/start_interval already covers slow first boots).

Verification

• go test ./internal/... — pass, including the new TestPodmanComposeProviderPrefersDockerCompose (both providers present → docker-compose), TestDockerComputeDownPinsStandalonePodmanCompose (Docker Compute down pins podman-compose with both present), TestDownKubernetesComposeModePrefersDockerCompose, and the keep on failure scenario in TestK3dCodexPreflightOrdering.
• node --test tests/integration/dev-up.test.mjs — 24 pass, including a new keep-on-failure case that proves the stack is kept, the hint is printed, and occ dev down still cleans it up.
• node --test tests/integration/dev-up-compose.test.mjs — 59 pass (the Podman fixture still exercises the podman-compose fallback).
• pnpm lint, pnpm format:check, pnpm docs:check, pnpm docs:check-length, pnpm check:workspace — clean.

Risks

Development tooling only. The Compose provider preference changes which binary the launcher invokes under Podman, and the keep-on-failure switch changes failure cleanup when explicitly set. No production configuration or behavior changes.

[clawsweeper]

🦞👀
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]

Codex review: needs real behavior proof before merge.

What this changes

This PR changes Podman Compose selection, development file permissions, PostgreSQL readiness, worker diagnostics, and failed-startup retention.

Example: Start the Compose control plane with Kubernetes Compute and OpenShell.

• Before: Routing files use mode 0600 and the existing helper assigns their container owner.
• After: Routing files use mode 0644, so the helper rejects them with “Mounted credentials must be private regular files” before the controller starts.

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The patch contains definite startup regressions and incomplete diagnostics; existing-state compatibility also remains unverified despite unchanged persisted formats.
Proof confidence	🧂 unranked krab (1/6)	Real behavior proof is necessary before merge. See Before merge.
Patch quality	🦪 silver shellfish (2/6)	6 actionable review findings remain.

Product

Kind: Bug fix · Worth it: Needs a maintainer decision · Fix scope: Partial
User problem: Developers using Podman cannot reliably start the Compose control plane or diagnose lifecycle failures.
Reason: The startup problem is useful work, but the new retention option needs an owner decision and the claimed file-mode preservation fix already exists in the base.

Merge readiness

⛔ Blocked before merge - 12 items remain

Keep this PR open for maintainer triage: the Podman repair has value, but the patch breaks routing-file preparation, restores a PostgreSQL readiness race, and leaves parts of the logging change ineffective.

Priority: P2
Reviewed head: e533e8333c93863a7686fbbad54c357b04da1c81
Owner decision: Required. See Decision needed.

Decision needed

• Question: Should this PR add OCC_DEVELOPMENT_KEEP_ON_FAILURE as a supported development setting?
• Recommendation: Keep this repair focused: Remove the new setting from this PR and decide failure-retention behavior with the owning startup work.
• Why: This changes the documented rollback contract through a new configuration option, and no human approval of that direction appears in the discussion.

Before merge

• Add real behavior proof - The supplied evidence reports conformance and static checks, but does not show the changed launcher and diagnostics running after the fix through occ dev up, first-agent, or the worker process; provide redacted terminal output or logs for the affected scenarios without IPs, keys, or non-public endpoints. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.
• Preserve 0600 for files handled by the routing ownership helper (P1) - With the Compose control plane and OpenShell selected, Up calls prepareComposeRoutingFiles before starting the controller and worker. Its assignComposeFileOwner helper rejects every file whose permissions are not exactly 0600. Writing gateway-api-key and gateway-ca.crt as 0644 therefore makes startup fail with “Mounted credentials must be private regular files” on Docker as well as Podman. Keep the private modes and use the existing helper to grant the actual container UID access.
• Keep the PostgreSQL readiness probe on TCP (P2) - The new psql command omits -h and connects to the temporary initialization server over its Unix socket. migrations/init-local.sql creates occ_migrator while that server is running, so the query can succeed before the entrypoint stops it and starts the final TCP server. Compose may then release migrate or return from up --wait while PostgreSQL is shutting down. This restores the race fixed by fix(test): wait for Postgres over TCP so init restarts can't race database creation #3112; retain the existing TCP readiness check.
• Carry retry diagnostics to an actual completion-log consumer (P2) - For a namespace operation that throws Error("connection failed"), the new message is attached to result but finalize selects only outcome, code, and observation-derived fields. The stop, deletion, and revision finalizers likewise never read result.message, and queue calls retain only code. Consequently these additions do not surface the advertised retry causes. Carry reviewed diagnostic fields through the existing emitters and logging boundary instead of attaching an unused result property.
• Keep fallback diagnostics valid when cause text is long (P2) - The fallback now appends the complete cause.message. A generic preparation error with 300 printable characters produces a message above the worker's 256-character limit, so validComputeFailureDiagnostic rejects the entire diagnostic and worker.compute-prepare-failed is not emitted. A multiline cause has the same effect. Previously the fixed fallback retained the classification and stage. Preserve those fields and omit or safely bound unsupported cause text before returning the diagnostic.
• Limit the retention setting's documentation to its implemented profile (P3) - The new row promises failed-startup retention for the Kubernetes profile generally. With OCC_DEVELOPMENT_CONTROL_PLANE=kubernetes, Up returns through upK3d before reaching the new condition, and upK3d still rolls back and removes state after failure. A developer relying on this setting loses the inspection state the documentation promises to retain. State that it applies only to the Compose-backed Kubernetes profile unless broader behavior is approved and implemented.
• Include the new supported setting in the environment cheat sheet (P3) - OCC_DEVELOPMENT_KEEP_ON_FAILURE is added to the supported development settings but omitted from docs/reference/cheatsheets/environment-variables.md. AGENTS.md explicitly requires that inventory to change with supported configuration variables, so readers using the environment inventory cannot discover this setting. Add a short entry linking to its owning development reference if the option is retained.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.
• Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.
• Explain or restore the original intent of PostgreSQL readiness - 313d65c: fix(test): wait for Postgres over TCP so init restarts can't race database creation #3112 deliberately selected TCP because the temporary initialization server can answer socket probes before shutting down.
• Explain or restore the original intent of Private routing-file permissions - 3414ccb: feat(openshell): support Codex PAT credentials #2048 deliberately retained 0600 and assigned actual container ownership, including proof that an unrelated UID could not read the files.
• Explain or restore the original intent of Compute preparation diagnostics - dd2b058: feat/openshell first agent codex #1400 supplied classified preparation diagnostics; the current worker accepts only messages of at most 256 printable characters.

Findings

• [P1] Preserve 0600 for files handled by the routing ownership helper — internal/occdev/gateway_k3d.go:533-535
• [P2] Keep the PostgreSQL readiness probe on TCP — compose.postgres.yaml:22
• [P2] Carry retry diagnostics to an actual completion-log consumer — apps/controller/src/worker.ts:2498-2503

Provenance

• PostgreSQL readiness changes intended behavior without addressing why it exists (313d65c: fix(test): wait for Postgres over TCP so init restarts can't race database creation #3112 deliberately selected TCP because the temporary initialization server can answer socket probes before shutting down.)
• Private routing-file permissions changes intended behavior without addressing why it exists (3414ccb: feat(openshell): support Codex PAT credentials #2048 deliberately retained 0600 and assigned actual container ownership, including proof that an unrelated UID could not read the files.)
• Compute preparation diagnostics changes intended behavior without addressing why it exists (dd2b058: feat/openshell first agent codex #1400 supplied classified preparation diagnostics; the current worker accepts only messages of at most 256 printable characters.)

Tests

• Missing end-to-end proof: No supplied shipped-entry-point evidence exercises repaired Podman startup, PostgreSQL first initialization, routing-file preparation, retry diagnostics, or existing-state reuse, and no base-failing/head-passing regression comparison is shown.

Agent review details

How this fits together

The development launcher provisions PostgreSQL and OCC in Compose, connects them to k3d workloads, and prepares private routing files; the worker executes queued lifecycle operations and emits operational diagnostics.

flowchart LR
  A[Development CLI] --> B[Compose provider]
  B --> C[PostgreSQL]
  B --> D[Routing file preparation]
  D --> E[OCC API and worker]
  E --> F[Kubernetes workloads]
  E --> G[Operational logs]

Loading

Technical review

Best possible solution:

Coordinate the Podman repair with the existing startup PR, retain private-file ownership and TCP readiness, and carry accepted diagnostics through the existing logging owner.

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

Source inspection establishes the introduced routing-file rejection and dropped diagnostic paths; no current-main Podman runtime reproduction was run.

Is this the best way to solve the issue?

The provider-selection repair addresses a reported limitation, but the existing ownership helper already solves non-root file access and main already protects PostgreSQL initialization with TCP readiness.

Full review comments:

• [P1] Preserve 0600 for files handled by the routing ownership helper — internal/occdev/gateway_k3d.go:533-535
  With the Compose control plane and OpenShell selected, Up calls prepareComposeRoutingFiles before starting the controller and worker. Its assignComposeFileOwner helper rejects every file whose permissions are not exactly 0600. Writing gateway-api-key and gateway-ca.crt as 0644 therefore makes startup fail with “Mounted credentials must be private regular files” on Docker as well as Podman. Keep the private modes and use the existing helper to grant the actual container UID access.
  Confidence: 1
• [P2] Keep the PostgreSQL readiness probe on TCP — compose.postgres.yaml:22
  The new psql command omits -h and connects to the temporary initialization server over its Unix socket. migrations/init-local.sql creates occ_migrator while that server is running, so the query can succeed before the entrypoint stops it and starts the final TCP server. Compose may then release migrate or return from up --wait while PostgreSQL is shutting down. This restores the race fixed by fix(test): wait for Postgres over TCP so init restarts can't race database creation #3112; retain the existing TCP readiness check.
  Confidence: 0.99
• [P2] Carry retry diagnostics to an actual completion-log consumer — apps/controller/src/worker.ts:2498-2503
  For a namespace operation that throws Error("connection failed"), the new message is attached to result but finalize selects only outcome, code, and observation-derived fields. The stop, deletion, and revision finalizers likewise never read result.message, and queue calls retain only code. Consequently these additions do not surface the advertised retry causes. Carry reviewed diagnostic fields through the existing emitters and logging boundary instead of attaching an unused result property.
  Confidence: 0.99
• [P2] Keep fallback diagnostics valid when cause text is long — apps/controller/src/drivers/compute/kubernetes/index.ts:782-785
  The fallback now appends the complete cause.message. A generic preparation error with 300 printable characters produces a message above the worker's 256-character limit, so validComputeFailureDiagnostic rejects the entire diagnostic and worker.compute-prepare-failed is not emitted. A multiline cause has the same effect. Previously the fixed fallback retained the classification and stage. Preserve those fields and omit or safely bound unsupported cause text before returning the diagnostic.
  Confidence: 0.98
• [P3] Limit the retention setting's documentation to its implemented profile — docs/reference/settings/development.md:40
  The new row promises failed-startup retention for the Kubernetes profile generally. With OCC_DEVELOPMENT_CONTROL_PLANE=kubernetes, Up returns through upK3d before reaching the new condition, and upK3d still rolls back and removes state after failure. A developer relying on this setting loses the inspection state the documentation promises to retain. State that it applies only to the Compose-backed Kubernetes profile unless broader behavior is approved and implemented.
  Confidence: 0.99
• [P3] Include the new supported setting in the environment cheat sheet — docs/reference/settings/development.md:40
  OCC_DEVELOPMENT_KEEP_ON_FAILURE is added to the supported development settings but omitted from docs/reference/cheatsheets/environment-variables.md. AGENTS.md explicitly requires that inventory to change with supported configuration variables, so readers using the environment inventory cannot discover this setting. Add a short entry linking to its owning development reference if the option is retained.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Podman Compose provider selection changes intended behavior with a stated reason (dd5729e: feat: unify development startup for Kubernetes compute #164 established the Go launcher and pinned Compose provider; later Kubernetes-only startup separated engine selection from Compose requirements.)
• Development file replacement keeps the original intent (3414ccb: The existing implementation already passes the original file mode to exclusiveWrite; this PR preserves that behavior while renaming the local variable.)
• Worker failure classification changes intended behavior with a stated reason (fix(compute): keep cancellations, broken connections and logged status on private secret writes #1488: The prior change carried sanitized HTTP status and fixed Status reason through completion logs while preserving failure classification and retry semantics.)
• Failed-startup rollback changes intended behavior with a stated reason (dd5729e: feat: unify development startup for Kubernetes compute #164 deliberately rolls back failed startup and cancellation; this PR explains the proposed inspection escape hatch.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

What I checked:

• Introduced changes: Reviewed the pinned merge-base-to-head hunks across all nine changed paths; the file-mode preservation hunk only renames a variable because preservation already exists in the base. (internal/occdev/repository_k3d.go:343, e533e8333c93)
• Existing private-file owner: assignComposeFileOwner explicitly rejects modes other than 0600; Up invokes it before starting the controller and worker. feat(openshell): support Codex PAT credentials #2048 introduced this owner and records allowed-reader and unrelated-UID denial verification. (internal/occdev/compose_routing.go:84, e533e8333c93)
• Readiness race already repaired: Current main retains the TCP probe. fix(test): wait for Postgres over TCP so init restarts can't race database creation #3112 documents the temporary socket-only server and reproduced shutdown race; fix(compose): give the postgres healthcheck a start period #3127 already added the slow-init start period. (compose.postgres.yaml:16, bd67c46d5e53)
• Retry text has no consumer: The new result.message properties are not read by the lifecycle finalizers, queue calls, or completion emitters; completion logs continue to select explicit fields. (apps/controller/src/worker.ts:2501, e533e8333c93)
• Diagnostic acceptance boundary: validComputeFailureDiagnostic rejects the entire diagnostic when its message exceeds 256 characters or contains control characters; the changed generic fallback appends unrestricted cause text. (apps/controller/src/worker.ts:856, e533e8333c93)
• Related Podman repair: fix(dev): support native rootful Podman startup and cleanup #3354 independently addresses the same unsupported project-directory option and broader native Podman startup and cleanup behavior through the existing launcher. (internal/occdev/command.go:138, bd67c46d5e53)

Likely related people:

• Kevin: Raw commit 3414ccb adds internal/occdev/compose_routing.go:84 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 3414ccb0d27a; files: internal/occdev/compose_routing.go)
• Derek Carr: Raw commit dd5729e adds internal/occdev/command.go:113 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: dd5729e195df; files: internal/occdev/command.go)

Labels

Label changes:

• add P2: The change affects development startup and diagnostics; the observed defects are significant but do not establish a current production outage.
• add merge-risk: 🚨 availability: The introduced file modes make Compose/OpenShell startup fail, and the socket probe can admit PostgreSQL before its initialization restart completes.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

• P2: The change affects development startup and diagnostics; the observed defects are significant but do not establish a current production outage.
• merge-risk: 🚨 availability: The introduced file modes make Compose/OpenShell startup fail, and the socket probe can admit PostgreSQL before its initialization restart completes.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

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.

Reviewed October 29, 2026, 5:42 AM ET / 09:42 UTC.

[freeqaz]

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

Thanks for splitting this out. I reviewed head e533e8333c. The podman-compose diagnosis is right: podman-compose 1.6.0 has no --project-directory, and composeCommand() always passes it. Several parts need changes before this can land, though. Two of them affect paths outside Podman.

Blocking

1. The 0644 routing files break the Compose + OpenShell profile on every engine. prepareComposeDevelopmentRouting now writes gateway-api-key and gateway-ca.crt as 0644. prepareComposeRoutingFiles then calls assignComposeFileOwner (internal/occdev/compose_routing.go). Before it chowns the files to the reader UID, it checks (stat.mode&0o777)!==0o600 and throws Mounted credentials must be private regular files. I ran that exact check against a 0644 file and it throws; against a 0600 file it passes. So occ dev up with OCC_DEVELOPMENT_SANDBOX_DRIVER=openshell in Compose mode now fails on Docker too. The OpenShell CI suite runs this profile (dev-up runs OCC in Compose with OpenShell and Kubernetes Compute in k3d), and go test ./internal/occdev/ passes because no unit test covers it. The chown step exists so these files can stay 0600 and still be readable by the non-root containers. Please drop the two mode changes. If rootful Podman really fails in assignComposeFileOwner, please share that error so we can fix the owner step instead.

2. occ dev down still requires podman-compose. internal/occdev/down.go still resolves only podman-compose for Compose-mode Podman state. On a host that has only docker-compose, which is the setup this PR targets, up now works but down fails with "executable file not found". With podman-compose installed, down still hits the same --project-directory problem. Please move the provider choice into one helper that both up and down use. Please also add a command_test.go case for "both providers present → docker-compose". The current table only puts podman-compose on PATH, so the new preference has no test.

3. The worker and Kubernetes Driver changes are production code, and they don't do what the PR describes.

   • The message added to the namespace reconcile, Agent stop and Agent deletion retry results is never read. DispatchResult has no message field. finalize, finalizeAgentStop and finalizeAgentDeletion pass only code to the queue and log. The log sanitizer also keeps message only for compute.preflight-warning and worker.compute-prepare-failed.
   • primaryFailureLogFields now puts the raw error text into reason. In this file reason is a one-word Kubernetes Status reason (LOGGED_STATUS_REASON). That branch only runs for errors with an HTTP status. A Kubernetes SDK ApiException message is always multi-line (HTTP-Code: …\nMessage: …\nBody: …), so the sanitizer's SAFE_STRING check drops it. I confirmed this through createWorkerLogEmitter.
   • preparationFailureDiagnostic now returns cause.message without a length bound. The Driver contract says describePrepareRevisionFailure must "never return raw errors" and must return only a "bounded, non-secret operator explanation". The worker's validComputeFailureDiagnostic rejects any message over 256 characters or with a control character, and that drops the whole worker.compute-prepare-failed event (code, stage and class too). The same file already guards this case with diagnosticMessage() ("a message it refuses would drop the whole diagnostic"). So with this change, long or multi-line causes now lose the diagnostic they had before. Messages that do fit are mostly dropped by the sanitizer anyway because of quotes and commas.

   Please take these changes out of this PR. Better retry diagnostics would be welcome as a separate change, but that change needs to go through the existing bounded paths (diagnosticMessage, loggedErrorText) and have a test. It also needs a maintainer to review it against the log-hygiene boundary.

4. The postgres healthcheck change also gates CI, and it can go healthy too early. Every PostgreSQL CI lane runs docker compose -f compose.postgres.yaml up -d --wait (scripts/ci/prepare.mjs) and then connects over TCP straight away. The official image's init phase runs a temporary server with listen_addresses=''. During that phase pg_isready -h 127.0.0.1 fails, which is the behaviour we want, so the premise in the PR description doesn't hold for the current check. The new check runs psql without -h, so it uses the Unix socket, which the temporary server does serve. CREATE ROLE occ_migrator is the first statement in init-local.sql. That means the service can report healthy while the init script is still running and before TCP is up. I reproduced this with Docker 28 and postgres:18.6 on a remote test VM. I added one init script after the role file (SELECT pg_sleep(4)) to stand in for a slow init tail on a loaded disk. With main's check, up --wait returned after 6-7 s and a TCP connection made right away succeeded (3/3). With this PR's check, up --wait returned after about 3 s, init hadn't finished, and the TCP connection made right away was refused (3/3). Without the delay, both checks passed 8/8 on an idle VM, so on CI this would show up as an intermittent failure under load. start_period/start_interval are already on main. If first boot on slow disks is the real problem, raising retries with the existing TCP pg_isready check covers it. If you do want a role check, add -h 127.0.0.1 so it only passes on the final server.

Other

• replaceDevelopmentFile on main already preserves the mode (info.Mode().Perm()). This hunk only renames the variable, so the description overstates it. The commit message also mentions an OpenShell client secret that isn't in the diff.
• OCC_DEVELOPMENT_KEEP_ON_FAILURE looks useful. When it is set, the copied service key (keyWritten && !state.KeyOwned) also stays on disk, and the error no longer says the state directory was kept for occ dev down. Please mention both in the settings row. The new variable also belongs in docs/reference/cheatsheets/environment-variables.md.
• The flow and reference docs still say Podman "pins standalone podman-compose" (docs/flows/docker-compose-development/startup.md, docs/reference/drivers/docker-compute.md, docs/testing/kubernetes.md). Please update them with the new preference. Please also say whether rootful Podman needs podman.socket enabled for the DOCKER_HOST route.
• In first-agent.mjs, availableInPath checks only that the file exists, not that it is executable. That's minor, but it differs from Go's exec.LookPath.
• "Risks: none … production configuration and behavior are untouched" isn't accurate. The PR changes worker.ts, the Kubernetes Compute Driver, and the healthcheck that CI's PostgreSQL lanes depend on.
• CI now runs on this head (fork runs approved after a check of the diff). PostgreSQL Application fails at postgres-worker-agent-revision.test.mjs:914, which expects no reason field in the logged activation failure. That is the primaryFailureLogFields change from point 3.

Suggested path: keep this PR to the provider preference (with down.go parity, a test and the docs) plus KEEP_ON_FAILURE, and drop the mode, healthcheck and worker/Driver hunks or move them to their own PRs.

@freeqaz-openai flagging this one for your review.

[jhjaggars]

Thanks for the thorough review — the diagnosis on every point was correct, and the suggested path is what I took. The branch is rebased onto current main, the worker/Driver and healthcheck commits are dropped, and the two remaining commits carry only the provider preference and KEEP_ON_FAILURE. Since that rewrites the branch, I pushed with --force-with-lease against the reviewed head.

Blocking

1. 0644 routing files — dropped in 0a0a55c3b (fix(dev): prefer docker-compose for Compose-mode Podman). gateway-ca.crt and gateway-api-key are back to 0600, and the replaceDevelopmentFile rename-only hunk is gone too. I did not hit an assignComposeFileOwner failure under rootful Podman, so no owner-step change was needed.

2. occ dev down parity — the provider choice now lives in one helper (composeProvider in internal/occdev/command.go) that both selectContainerEngine and Down use, so up and down agree on every host. Added TestPodmanComposeProviderPrefersDockerCompose in internal/occdev/command_test.go: with both providers on PATH, selection pins docker-compose; the existing table still covers the podman-compose-only case, and the dev-up fixture suite keeps proving the fallback end to end. Also in 0a0a55c3b.

3. Worker and Kubernetes Driver changes — removed entirely (the PostgreSQL Application failure at postgres-worker-agent-revision.test.mjs:914 was exactly the primaryFailureLogFields change, and it is gone with it). If the retry diagnostics are still wanted I'll propose them separately through the bounded paths (diagnosticMessage, loggedErrorText) with a test, for maintainer review against the log-hygiene boundary.

4. postgres healthcheck — reverted to main's TCP pg_isready check; main's start_period/start_interval already cover slow first boots, so compose.postgres.yaml is now identical to main.

Other

• The settings row now says the copied service key also stays on disk, and the launcher prints an explicit cleanup hint (OCC_DEVELOPMENT_KEEP_ON_FAILURE is set; kept the failed stack ... Run occ dev down ...) instead of leaving the operator without the rollback error's pointer — 10f13d52a (feat(dev): keep a failed development stack for inspection). The variable is also in docs/reference/cheatsheets/environment-variables.md. The switch now covers both the Compose control-plane and Kubernetes-only profiles, with a keep on failure scenario in TestK3dCodexPreflightOrdering and a dev-up integration case proving the stack is kept and occ dev down still cleans it up.
• Docs updated in 0a0a55c3b: docs/flows/docker-compose-development/startup.md and docs/reference/drivers/docker-compute.md now state the docker-compose-over-podman-compose preference and that rootful Podman on Linux must expose its API socket (e.g. systemctl enable --now podman.socket) for the DOCKER_HOST route; the flow overview's assumptions line matches. docs/testing/kubernetes.md already said "Docker Compose or podman-compose" without pinning language, so it needed no change.
• availableInPath in first-agent.mjs now checks the executable bit (accessSync with X_OK), matching exec.LookPath.
• The PR description is rewritten: the inaccurate risks claim and the client-secret mention are gone, and the dropped hunks are called out explicitly.

[freeqaz]

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

Thanks for narrowing this down. I re-reviewed head 10f13d52aa. The 0644 routing-file modes, the worker and Kubernetes Driver hunks, and the healthcheck change are all gone, so the earlier blocking points no longer apply. CI Required and First Agent Smoke are green on this head. The branch merges cleanly with current main.

What I checked

• up and down now use the same composeProvider(). TestPodmanComposeProviderPrefersDockerCompose covers the case where both providers are installed.
• go test ./internal/occdev/... passes on a merge with main. tests/integration/dev-up.test.mjs passed 24/24 on a remote test VM.
• I ran a few mutants. Each of these fails a test:
  • reversing the provider order
  • disabling KEEP_ON_FAILURE in the Compose flow
  • disabling KEEP_ON_FAILURE in the k3d flow
• availableInPath in first-agent.mjs now checks X_OK.
• The settings row and the cheat sheet both mention that the copied service key is kept. The new cleanup hint names occ dev down.

One docs correction left

This PR changes only the Go occ dev up/occ dev down path, which serves Kubernetes Compute. Docker Compute still runs through scripts/dev-up itself. That script still requires and pins podman-compose (require_command podman-compose at line 248, and the prerequisite message at line 340). It never passes --project-directory, so it works as before. Three places now describe the Docker Compute path as preferring docker-compose:

• docs/reference/drivers/docker-compute.md: the Requirements bullet and the "scripts/dev-up is the supported Podman entry point" paragraph.
• docs/flows/docker-compose-development/startup.md, step 1 (the scripts/dev-up host preflight).
• docs/flows/docker-compose-development.md, the Assumptions line.

Please limit the new provider sentence to Kubernetes Compute (occ dev up/occ dev down). For Docker Compute, please keep saying it needs podman-compose.

Optional

• A mutant that puts down.go back on exec.LookPath("podman-compose") passes every test. A small Down case with only docker-compose on PATH would cover the parity that this PR is about.
• In the new dev-up test, the key-mode assertion sits behind if (existsSync(keyOutput)), and that branch never runs. copyAndVerifyKey writes --key-output only after GetInstallation succeeds, so the api-unauthorized scenario fails before any key is copied. As a result, no test covers the "copied service key is kept" part. It would need a scenario that fails after keyWritten = true, for example the OpenShell Namespace wait. Alternatively, drop the conditional.

What's left from our side is the docs correction above. Once that's in, this is ready to merge.

[freeqaz]

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

Thanks for the update. I re-reviewed head 44a189d28e. Since 10f13d52aa the branch only merged main, so the PR's own diff is unchanged. CI Required is green, and the branch still merges cleanly with current main.

What passes

• go test ./internal/occdev/... passes on a merge with current main. tests/integration/dev-up.test.mjs and dev-up-compose.test.mjs passed 83/83 on a remote test VM.
• Mutants: reversing the provider order fails TestPodmanComposeProviderPrefersDockerCompose. Disabling KEEP_ON_FAILURE in upK3d fails TestK3dCodexPreflightOrdering. Disabling it in Up fails the new dev-up test.

1. Bug: Docker Compute occ dev down now uses a different provider than scripts/dev-up

The Docker Compute branch of Down (internal/occdev/down.go, case "docker") calls r.selectEngine. That goes through selectContainerEngine(..., true), which now calls composeProvider(). So on a Podman host with both docker-compose and podman-compose installed, scripts/dev-down / occ dev down stops the stack with docker-compose. But scripts/dev-up still starts the Docker Compute stack with podman-compose: it runs require_command podman-compose and exports PODMAN_COMPOSE_PROVIDER="$(command -v podman-compose)". Before this PR, both paths used podman-compose.

I checked whether this matters with a small two-service Compose file (a long-running service, a one-shot service and a named volume) on rootless Podman 6.1.4:

• podman compose up -d with PODMAN_COMPOSE_PROVIDER=podman-compose (1.6.0) started the stack.
• podman compose down --volumes with PODMAN_COMPOSE_PROVIDER=docker-compose (v5.3.1) exited 0 but removed nothing. Both containers were still there, the volume reported "Resource is still in use", and the network stayed.
• down --volumes with podman-compose then cleaned everything up.

So on such a host, occ dev down prints "Stopped Docker-compatible development stack." and leaves the stack running.

Suggested fix: use the new preference only on the Kubernetes Compute Compose path, where Go owns both up and down. Keep the Docker Compute down on podman-compose so it matches scripts/dev-up. One way is to give selectContainerEngine the provider lookup to use, or a flag for it. A Down test case with Docker Compute on Podman and both providers on PATH would cover it.

2. Docs (the point from the last review, still open)

The docs still say the Docker Compute entry point prefers docker-compose, but scripts/dev-up still requires and pins podman-compose:

• docs/reference/drivers/docker-compute.md: the Requirements bullet, and the "scripts/dev-up is the supported Podman entry point ... prefers the docker-compose provider" paragraph.
• docs/flows/docker-compose-development/startup.md, step 1: the paragraph that starts "Podman requires yq v4 and a Compose provider".
• docs/flows/docker-compose-development.md, the Assumptions line.

Please limit the provider preference to Kubernetes Compute (occ dev up/occ dev down). For Docker Compute, please keep saying that it needs podman-compose. With fix 1 in place, "occ dev down resolves the same preference" holds only for Kubernetes Compute, so please scope that sentence the same way.

Optional (unchanged from last time)

• A mutant that puts the Kubernetes down.go path back on exec.LookPath("podman-compose") still passes every test.
• The new dev-up test's key-mode check (if (existsSync(keyOutput))) never runs in the api-unauthorized scenario, because the key is copied only after GetInstallation succeeds.
• The provider choice in scripts/first-agent.mjs (availableInPath) has no test.

Once 1 and 2 are fixed, this looks ready to merge from our side.

[jhjaggars]

Both points fixed in a48ef883b (fix(dev): keep Docker Compute down on the standalone provider), plus the two optional coverage gaps you flagged. Normal push on top of the merge commit; no history rewrite.

1. Docker Compute occ dev down provider mismatch — selectContainerEngine now takes the provider lookup to use. The Kubernetes-profile path (selectEngine) keeps composeProvider() (docker-compose preferred), while Docker Compute down goes through a new selectDockerComputeEngine that resolves the standalone podman-compose scripts/dev-up requires and pins, so down always stops the stack with the provider that started it. The Kubernetes Down branch (Compose-mode state) still resolves composeProvider() directly.

2. Docs scoping — docker-compute.md's requirements bullet and entry-point paragraph again say Docker Compute requires and pins standalone podman-compose (now also saying occ dev down matches it). startup.md step 1 states the split explicitly: Docker Compute requires podman-compose; the Kubernetes-profile Compose command that occ dev up/occ dev down drive prefers docker-compose with the podman-compose fallback. The flow overview's assumptions line notes Docker Compute requires podman-compose. The "resolves the same preference" sentence is gone; the equivalent claim is now scoped to the Kubernetes-profile command.

Optional items

• The Kubernetes down.go mutant is now covered: TestDownKubernetesComposeModePrefersDockerCompose runs Down against recorded Compose-mode state with both providers on PATH, and the fixture's engine exits 97 unless PODMAN_COMPOSE_PROVIDER is the docker-compose path. I verified the mutant (reverting that branch to LookPath("podman-compose")) fails the test. TestDockerComputeDownPinsStandalonePodmanCompose covers the Docker Compute side with both providers present, per your suggestion.
• The dead if (existsSync(keyOutput)) branch is replaced with assert.rejects(stat(keyOutput), { code: "ENOENT" }) — the api-unauthorized failure happens before the service key is copied, so the kept stack provably holds no key material.
• availableInPath in first-agent.mjs remains untested; it is a four-line PATH scan replayed only by the first-agent harness. Happy to add coverage if you want it.

Verification on a48ef883b: go test ./internal/... pass; dev-up.test.mjs + dev-up-compose.test.mjs 83/83 pass; pnpm lint, format:check, docs:check, docs:check-length, check:workspace clean.

[freeqaz]

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

Thanks for the fix. I re-reviewed head a48ef883bc. Both points from my last comment are fixed, and this looks ready to merge from our side.

1. Docker Compute occ dev down provider: fixed

• Down now selects the engine for Docker Compute through selectDockerComputeEngine, which pins standalone podman-compose, the same provider that scripts/dev-up requires and pins. The Kubernetes Compute paths (occ dev up through selectEngine, and the Kubernetes Down branch) keep the docker-compose preference through composeProvider.
• I repeated the earlier check, this time through the real CLI. I built bin/occ from this head merged with current main and used rootless Podman 6.1.4 with both podman-compose 1.6.0 and docker-compose v5.3.1 on PATH. I started a small two-service stack with a named volume through podman compose with PODMAN_COMPOSE_PROVIDER pinned to podman-compose, as scripts/dev-up does. Then I ran OCC_DEVELOPMENT_COMPUTE_DRIVER=docker OCC_DEVELOPMENT_CONTAINER_ENGINE=podman occ dev down --volumes:
  • This head ran podman-compose and removed both containers, the volume and the network.
  • The same binary with Down switched back to selectEngine ran docker-compose, printed "Stopped Docker-compatible development stack." and left the containers, volume and network in place.

2. Docs: fixed

docker-compute.md, step 1 of startup.md and the Assumptions line of docker-compose-development.md now say that Docker Compute requires podman-compose. They give the docker-compose preference only for the Kubernetes-profile Compose command, for both occ dev up and occ dev down. This matches the code.

Checks

• go test ./internal/occdev/... passes on a merge with current main. tests/integration/dev-up.test.mjs and dev-up-compose.test.mjs passed 83/83 on a remote test VM. That includes the key-mode check, which now asserts that no key exists.
• CI Required is green on the head, and the branch merges cleanly with main.
• Mutants:
  • The one that survived last time (the Kubernetes down.go path back on podman-compose) now fails TestDownKubernetesComposeModePrefersDockerCompose.
  • Reversing the provider order fails TestPodmanComposeProviderPrefersDockerCompose, and so does making selectEngine use podmanComposeProvider.
  • Making selectDockerComputeEngine use composeProvider fails TestDockerComputeDownPinsStandalonePodmanCompose.

Optional, not blocking

• One mutant still survives: switching the Docker Compute branch of Down back from selectDockerComputeEngine to selectEngine. The new test calls the helper directly, so the fix's own wiring in Down is not covered. A Down case with Docker Compute, Podman, and both providers on PATH, whose fake engine checks PODMAN_COMPOSE_PROVIDER, would catch it. That would be like your new Kubernetes Down test.