Containerized provider login: login panes and every provider-CLI invocation become podman run against the master store (ADR-0057) #234

Merged

dominik.polakovics merged 1 commit from afk/206 into main

2026-07-25 20:11:44 +02:00

dominik.polakovics commented

2026-07-25 19:41:45 +02:00

Owner

Design (per the grilled 2026-07-25 brief + ADR-0057)

Seam — a decorated tmuxx.SessionRunner (providercli.LoginRunner): for login sessions only, the adapter's CLI argv is wrapped into the podman run pane command; the adapters' scrape/paste/poll/teardown flows are untouched. The decorator's Stop runs the podman rm backstop and wipes the per-attempt scratch HOME under <state>/logins/.
The non-interactive CLI surface rides the same machinery — this is the completion the acceptance criteria force: the spawn gate forces AuthStatus before every spawn, login completion polls it, and the refresh loop pokes the CLI. All adapter exec sites moved behind an injectable provider.CLIRunner (host default preserves pre-seam behavior byte-for-byte; providercli.ContainerCLI runs each invocation as an ephemeral non-tty podman run --rm). podman exit 125/126/127 is classified as could-not-run, never as a CLI verdict — a missing image can never read as "logged out".
Master store — mount source resolved through the provider-declared MasterStoreSpec (the same override-aware resolver every other master-store op uses), so $CLAUDE_CONFIG_DIR/$CODEX_HOME are honored in container mode (the #211 criterion, conformance-pinned); the config-dir var is pinned explicitly inside the container.
Trigger/image/limits — config-driven, no fallback; refusals (red/unfinished preflight, missing tools image, missing global --container-image) wrap the new provider.ErrLoginUnavailable and surface as a 400 with the actionable text verbatim. Global dev image only (login is repo-less); global container_* limit rows; per-attempt EnsureImage. No new knobs, no new flags.
Lifecycle — login containers use the deterministic labrun- name derived from the session name, so a pending login survives a restart (idempotent re-join) and the startup orphan sweep accounts for it by construction; ephemeral CLI containers are labrun--prefixed so wedged ones are reaped at the next boot.

Verification

Unit coverage: login/CLI podman argv goldens (podmanx + providercli), all refusal paths, decorator pass-through, Stop backstop + scratch wipe, exit-code classification, env/dir re-anchoring, preflight wait, conformance obligations for MasterStoreSpec, HostCLI byte-for-byte semantics, httpapi 400 mapping. Full suite: 46 packages green; vet + gofmt clean (golangci-lint unavailable in this sandbox — CI runs it).
Podman-gated integration tests (LAB_TEST_PODMAN=1, model of podmanx's): a real containerized login pane driven through real tmux — URL scraped via CapturePane, a write landing in the host master dir through the rw bind, Stop reaping container + scratch — plus a ContainerCLI round-trip. They skip where rootless podman is absent (this sandbox, CI).
A multi-angle review pass ran over the full diff; the four actionable findings (codex boot-probe serialization, duplicated prefix-rewrite discipline, triplicated limit defaults, a mid-rune stderr cut) are fixed in this PR.
Post-merge (maintainer, per the issue brief): the real-OAuth manual matrix — claude login, codex device login, restart-mid-login resume, a subsequent container run authenticating, and a CLI-less host.

Notes for review

Known accepted consequences (documented in ADR-0057): every forced auth check on a container host is a container spin (bounded by the 30s status cache); codex's boot catalog probe can serve the compiled-in fallback on a cold host (graceful, logged, mitigated by running the version+models probes concurrently); a server configured with only per-repo dev images and no global --container-image gets the knob-naming refusal for login/CLI ops by design.
Possible follow-up (not done, to keep the diff bounded): sharing the two byte-identical refusal strings between internal/instance and internal/providercli as exported consts.

Closes #206

With container wiring present, provider login sessions and every non-interactive provider-CLI invocation (auth status, logout, the ADR-0055 refresh poke, codex's catalog probe) now run in containers from the agent-tools image against the rw-mounted master credential store — a container-mode host needs no provider CLIs on PATH. Host-mode login stays byte-for-byte unchanged on servers with no container config, and there is never a silent host fallback. ## Design (per the grilled 2026-07-25 brief + ADR-0057) - **Seam** — a decorated `tmuxx.SessionRunner` (`providercli.LoginRunner`): for login sessions only, the adapter's CLI argv is wrapped into the `podman run` pane command; the adapters' scrape/paste/poll/teardown flows are untouched. The decorator's Stop runs the `podman rm` backstop and wipes the per-attempt scratch HOME under `<state>/logins/`. - **The non-interactive CLI surface rides the same machinery** — this is the completion the acceptance criteria force: the spawn gate forces `AuthStatus` before every spawn, login completion polls it, and the refresh loop pokes the CLI. All adapter exec sites moved behind an injectable `provider.CLIRunner` (host default preserves pre-seam behavior byte-for-byte; `providercli.ContainerCLI` runs each invocation as an ephemeral non-tty `podman run --rm`). podman exit 125/126/127 is classified as could-not-run, never as a CLI verdict — a missing image can never read as "logged out". - **Master store** — mount source resolved through the provider-declared `MasterStoreSpec` (the same override-aware resolver every other master-store op uses), so `$CLAUDE_CONFIG_DIR`/`$CODEX_HOME` are honored in container mode (the #211 criterion, conformance-pinned); the config-dir var is pinned explicitly inside the container. - **Trigger/image/limits** — config-driven, no fallback; refusals (red/unfinished preflight, missing tools image, missing global `--container-image`) wrap the new `provider.ErrLoginUnavailable` and surface as a 400 with the actionable text verbatim. Global dev image only (login is repo-less); global `container_*` limit rows; per-attempt `EnsureImage`. No new knobs, no new flags. - **Lifecycle** — login containers use the deterministic `labrun-` name derived from the session name, so a pending login survives a restart (idempotent re-join) and the startup orphan sweep accounts for it by construction; ephemeral CLI containers are `labrun-`-prefixed so wedged ones are reaped at the next boot. ## Verification - Unit coverage: login/CLI podman argv goldens (podmanx + providercli), all refusal paths, decorator pass-through, Stop backstop + scratch wipe, exit-code classification, env/dir re-anchoring, preflight wait, conformance obligations for `MasterStoreSpec`, HostCLI byte-for-byte semantics, httpapi 400 mapping. Full suite: 46 packages green; vet + gofmt clean (golangci-lint unavailable in this sandbox — CI runs it). - Podman-gated integration tests (`LAB_TEST_PODMAN=1`, model of podmanx's): a real containerized login pane driven through real tmux — URL scraped via CapturePane, a write landing in the host master dir through the rw bind, Stop reaping container + scratch — plus a ContainerCLI round-trip. They skip where rootless podman is absent (this sandbox, CI). - A multi-angle review pass ran over the full diff; the four actionable findings (codex boot-probe serialization, duplicated prefix-rewrite discipline, triplicated limit defaults, a mid-rune stderr cut) are fixed in this PR. - **Post-merge (maintainer, per the issue brief):** the real-OAuth manual matrix — claude login, codex device login, restart-mid-login resume, a subsequent container run authenticating, and a CLI-less host. ## Notes for review - Known accepted consequences (documented in ADR-0057): every forced auth check on a container host is a container spin (bounded by the 30s status cache); codex's boot catalog probe can serve the compiled-in fallback on a cold host (graceful, logged, mitigated by running the version+models probes concurrently); a server configured with only per-repo dev images and no global `--container-image` gets the knob-naming refusal for login/CLI ops by design. - Possible follow-up (not done, to keep the diff bounded): sharing the two byte-identical refusal strings between `internal/instance` and `internal/providercli` as exported consts. Closes #206

dominik.polakovics added 1 commit

2026-07-25 19:41:45 +02:00

feat(provider,runner): containerize provider login and the provider-CLI surface (issue #206 )

ci / native (pull_request) Successful in 7m52s

Details

3f8f7206b5

Container-mode hosts needed every provider CLI installed just to log in
and stay logged in: the login pane ran the CLI from the host PATH, and
the adapters shelled out for auth status (the spawn gate and the login
completion poll), logout, the ADR-0055 refresh poke, and codex's catalog
probe. With container wiring present, all of it now runs in containers
against the rw-mounted master credential store — the agent-tools image
is the only CLI distribution, and there is never a host-CLI fallback
(ADR-0057). A server with no container config keeps host-mode login
byte-for-byte unchanged.

- provider.AgentProvider grows MasterStoreSpec() — the override-aware
  master-store declaration ($CLAUDE_CONFIG_DIR/$CODEX_HOME honored for
  the mount source, the #211 criterion, conformance-pinned) — and the
  CLIRunner seam (CLIInvocation, HostCLI default): adapters describe
  WHAT to run, the runner decides where. Every adapter exec site rides
  it; the nil default is byte-for-byte the pre-seam host exec.
- podmanx.RunSpec renders three shapes: run (golden-pinned, unchanged),
  login pane (per-attempt scratch HOME + nested master-store bind, -it),
  and non-interactive CLI (NoTTY, store bind only, overlay HOME).
  RewritePathPrefix/RewriteEnvPrefix generalize the exact-or-path-prefix
  re-anchoring discipline into one shared function.
- internal/providercli: LoginRunner decorates the SessionRunner so a
  login session's pane command becomes `podman run` — refusals wrap the
  new provider.ErrLoginUnavailable (httpapi maps it to a 400 carrying
  the actionable text verbatim), Stop runs the `podman rm` backstop and
  wipes the scratch HOME under <state>/logins. ContainerCLI runs each
  CLI invocation as an ephemeral non-tty `podman run --rm`: ctx-bounded
  preflight wait for boot races, podman exit 125/126/127 classified as
  could-not-run so a missing image never reads as logged out.
- Wiring: with container config present each adapter gets the decorated
  runner + containerized CLI built from its package-level MasterStore
  resolver (the same one the adapter's method uses); the global dev
  image and global container_* limit rows apply (login is repo-less);
  instance/afk/reconcile keep the raw runner. store exports the
  container-limit defaults as the single source for every fallback.
- Login containers carry the deterministic labrun- name, so the startup
  orphan sweep accounts for a live login attempt by construction;
  ephemeral CLI containers are labrun--prefixed for the same sweep.
- docs: ADR-0057; ops.md — container hosts need no provider CLIs on
  PATH, agentPackages values may be null, <state>/logins in the state
  layout and backup exclusions.
- Podman-gated integration tests skip without LAB_TEST_PODMAN=1; the
  real-OAuth manual matrix (claude login, codex device login,
  restart-mid-login resume, subsequent container run, CLI-less host)
  stays post-merge maintainer verification per the issue brief.

Closes #206

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

dominik.polakovics commented

2026-07-25 19:51:48 +02:00

Author

Owner

[autoland] verdict: pass

PASS with CONCERNS (non-blocking). Signal: CI aggregate success — 'ci / native (pull_request)' run #310 (7m52s), the repo's full build/test/vet/gofmt/golangci-lint gate; not re-run.

Conventions verified:

Closes #206 linkage present (required for afk/206 — met).
Diff scope matches issue #206 acceptance criteria; no drive-by changes. The store.DefaultContainer* consolidation and httpapi 400 ErrLoginUnavailable mapping are on-topic (the former is one of the review findings this PR fixed).

CONCERN (non-blocking): the PR TITLE 'Containerized provider login: …' is not Conventional-Commits shaped (no type(scope): prefix). The single commit is conventional (feat(provider,runner): …), but Forgejo embeds the PR title as the merge-commit subject, and every recent merge here (#233 fix(deploy):, #231 feat(nix):) uses a conventional title — so merging as-is breaks that history pattern. Not fixable inline (no labctl pr title-edit; not a commit-level fix). Suggest retitling to e.g. 'feat(provider,runner): containerize provider login and the provider-CLI surface' before merge.

Per autoland policy, any CONCERNS caps at approve-only: validated, NOT merged — awaiting a human to retitle+merge or accept.

[autoland] verdict: pass PASS with CONCERNS (non-blocking). Signal: CI aggregate success — 'ci / native (pull_request)' run #310 (7m52s), the repo's full build/test/vet/gofmt/golangci-lint gate; not re-run. Conventions verified: - Closes #206 linkage present (required for afk/206 — met). - Diff scope matches issue #206 acceptance criteria; no drive-by changes. The store.DefaultContainer* consolidation and httpapi 400 ErrLoginUnavailable mapping are on-topic (the former is one of the review findings this PR fixed). CONCERN (non-blocking): the PR TITLE 'Containerized provider login: …' is not Conventional-Commits shaped (no type(scope): prefix). The single commit is conventional (feat(provider,runner): …), but Forgejo embeds the PR title as the merge-commit subject, and every recent merge here (#233 fix(deploy):, #231 feat(nix):) uses a conventional title — so merging as-is breaks that history pattern. Not fixable inline (no labctl pr title-edit; not a commit-level fix). Suggest retitling to e.g. 'feat(provider,runner): containerize provider login and the provider-CLI surface' before merge. Per autoland policy, any CONCERNS caps at approve-only: validated, NOT merged — awaiting a human to retitle+merge or accept.

dominik.polakovics merged commit 188399965b into main

2026-07-25 20:11:44 +02:00

dominik.polakovics deleted branch afk/206

2026-07-25 20:11:44 +02:00

dominik.polakovics referenced this pull request from a commit

2026-07-25 20:11:45 +02:00

Merge pull request 'Containerized provider login: login panes and every provider-CLI invocation become podman run against the master store (ADR-0057)' (#234) from afk/206 into main