Skip to content

feat: set-up — /etc/environment + /etc/profile root patches (PR-6c)#38

Merged
pofallon merged 5 commits into
mainfrom
pr6c-set-up-etc-patches
May 25, 2026
Merged

feat: set-up — /etc/environment + /etc/profile root patches (PR-6c)#38
pofallon merged 5 commits into
mainfrom
pr6c-set-up-etc-patches

Conversation

@pofallon
Copy link
Copy Markdown
Contributor

@pofallon pofallon commented May 25, 2026

Follow-up to PR-6b (#37). Implements spec §5 phase 3a: append-only root patches to `/etc/environment` and `/etc/profile` so login shells inherit the resolved container env / PATH. Both patches are guarded by marker files under `--container-system-data-folder` (default `/var/devcontainer/`) so re-running set-up against the same container is a no-op.

With this PR, the `set-up` subcommand is functionally complete for spec §5. The only deferred item is the live-container `${containerEnv:VAR}` second-pass substitution — an enhancement, not a behavior gap.

What this PR adds

  • New CLI flag: `--container-system-data-folder ` (spec §6, default `/var/devcontainer`).
  • `apply_etc_patches` wires both patches before the lifecycle hook phase (spec §5 puts system patches in phase 3a). Skipped when `--skip-post-create` is set, mirroring upstream's "post-create is the umbrella for user customization" semantics.
  • `build_etc_environment_patch_script`: emits a shell script that appends a `# >>> deacon set-up >>>` ... `# <<< deacon set-up <<<` block of `KEY="VALUE"` lines to `/etc/environment`. Escapes `\` and `"` so the PAM parser doesn't choke. Touches the marker only after a successful append. Empty env → no-op (does NOT touch the marker, so a later run with a populated config still patches).
  • `build_etc_profile_patch_script`: emits a shell script that re-exports PATH from `/etc/environment` inside a delimited block. This is the spec-§16 design-decision recipe — without it the env appended in `/etc/environment` doesn't reach login shells.
  • `collect_env_pairs`: merges `containerEnv` + `remoteEnv` + CLI `--remote-env` overlays into a sorted, deterministic list. CLI wins on key conflicts. `None`-valued `remote_env` entries are dropped.
  • Best-effort: any non-zero exit from the patch shell (no root, read-only `/etc`, missing `sh`, etc.) emits a WARN and proceeds. Spec §9: system-level patches "do not abort set-up unless critical".

Tests

10 new unit tests pinning the script-builder behavior:

  • Empty / idempotent paths: `returns_noop_when_env_empty`, `short_circuits_when_marker_present`, `short_circuits_on_marker`
  • Output shape: `writes_sorted_env_block`, `wraps_block_in_delimiters` (×2), `reexports_path_from_environment`
  • Robustness: `escapes_special_chars_in_values` (`\`, `"`)
  • Env collection: `merges_config_remote_and_cli` (CLI wins, `None` dropped), `returns_sorted_output`

Total set-up tests: 27 pass (14 from PR-6a + 3 from PR-6b + 10 new).

Verification:

  • `cargo fmt --all -- --check`
  • `cargo clippy --all-targets -- -D warnings`
  • `cargo test -p deacon --lib` → 240 pass (no regression)
  • CI green
  • End-to-end smoke against a real container — deferred to the integration-test follow-up; the unit tests pin the shell-script generation precisely

Refs

🤖 Generated with Claude Code

pofallon and others added 3 commits May 25, 2026 14:13
@pofallon @claude
feat: set-up subcommand — MVP scaffold + lifecycle hooks (PR-6a)
940c409 
Implements the core value of `set-up` per
`docs/subcommand-specs/set-up/SPEC.md`: convert an already-running container
into a DevContainer by applying configuration + image metadata and executing
lifecycle hooks, emitting a single-line JSON result on stdout.

## CLI surface

`deacon set-up --container-id <id> [--config <path>] [--skip-post-create]
[--skip-non-blocking-commands] [--remote-env NAME=VALUE]...
[--include-configuration] [--include-merged-configuration]
[--container-data-folder <path>]`

## What this PR includes

- `--container-id` resolution + inspect validation. Missing container fails
  with the upstream-aligned summary `"Dev container not found."`
- Optional `--config` load via `ConfigLoader::load_with_extends` (extends
  chain honored per CLAUDE.md). Missing path fails with
  `"Dev container config (<path>) not found."`
- Image-metadata extraction from the container's `devcontainer.metadata`
  label. Tolerates BOTH the JSON-array form (PR-2 / #27) and the
  single-object form for older images.
- Config merge: file config wins over image metadata on scalar fields
  (spec §4 `mergeConfiguration(config.config, imageMetadata)`).
- Variable substitution for both `configuration` and `mergedConfiguration`.
- Lifecycle hook execution via `ContainerLifecycle` (onCreate →
  updateContent → postCreate → postStart → postAttach), gated by
  `--skip-post-create` (skips ALL phases per spec §2) and
  `--skip-non-blocking-commands` (stops after the configured `waitFor`).
- JSON output per spec §10: `{outcome: "success", configuration?,
  mergedConfiguration?}`. `containerId` is intentionally excluded (spec
  §16 design decision).

## Deferred to PR-6b

- `/etc/environment` + `/etc/profile` root-side patches with system markers
  under `/var/devcontainer/`
- Dotfiles installer with target-path marker (would reuse
  `crates/deacon/src/commands/up/dotfiles.rs`)
- A second substitution pass against the live container environment
  (`${containerEnv:VAR}`) — current pass uses the configured
  `container_env`, not a live `docker exec` env probe

These are spec §5 phases 3a and 3c; both marked "best-effort" with graceful
fallback on failure. Splitting them out keeps PR-6a reviewable.

## Tests

14 new unit tests in `set_up::tests`:
- `--remote-env` parsing (accepts `NAME=VALUE`, rejects malformed input)
- `--config` loading (default when absent, error when missing path)
- Image-metadata label parsing (missing → None, array form, single-object
  form, invalid JSON → error)
- Config merging (file wins over metadata; file-only when no metadata)
- Argument defaults and JSON-result shape (outcome field, optional fields)

Verification:
- `cargo fmt --all -- --check`
- `cargo clippy --all-targets -- -D warnings`
- `cargo test -p deacon --lib` → 227 pass
- `make test-nextest-fast` → 1927 pass (no regression)

Refs: issue #34 (Tier 1 progress tracker), plan
`/home/vscode/.claude/plans/let-s-come-up-with-recursive-sutherland.md`
(PR-6 sequencing rationale).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pofallon @claude
feat: set-up — wire dotfiles installer (PR-6b)
a41027d 
Follow-up to PR-6a (#36). Adds the three `--dotfiles-*` flags from spec §2
to the `set-up` CLI surface and threads them as a `DotfilesConfig` into
`ContainerLifecycle`. The lifecycle helper already implements clone +
auto-detected installer (`install.sh` / `bootstrap` / `setup` / `script/*`)
+ target-path marker for idempotency — set-up just needed to opt in.

## New flags

- `--dotfiles-repository <URL or owner/repo>`
- `--dotfiles-install-command <string>` (overrides auto-detection)
- `--dotfiles-target-path <path>` (defaults to `~/dotfiles`)

No repository → no clone. The lifecycle helper's marker-at-target-path
guard provides idempotency across re-runs (spec §6, §16 design decision).

## Tests

3 new unit tests in `set_up::tests::build_dotfiles_config_*`:

- `returns_none_when_no_repository` — no repo URL means no opt-in, even if
  the other flags are set
- `forwards_all_three_fields` — all flags round-trip into `DotfilesConfig`
- `leaves_defaults_to_lifecycle_helper` — `target_path` / `install_command`
  stay `None` so the helper computes its standard defaults

Verification:
- `cargo fmt --all -- --check`
- `cargo clippy --all-targets -- -D warnings`
- `cargo test -p deacon --lib set_up` → 17 pass (14 from PR-6a + 3 new)

## Deferred to PR-6c

- `/etc/environment` + `/etc/profile` root-side patches with system markers
  under `/var/devcontainer/` (spec §5 phase 3a; net-new code that needs
  root exec + heredoc plumbing)
- Live-container `${containerEnv:VAR}` second-pass substitution

Refs: issue #34 (Tier 1 progress tracker); PR-6a (#36).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pofallon @claude
feat: set-up — /etc/environment + /etc/profile root patches (PR-6c)
571f291 
Follow-up to PR-6b (#37). Implements spec §5 phase 3a: append-only root
patches to `/etc/environment` and `/etc/profile` so login shells inherit
the resolved container env / PATH. Both patches are guarded by marker
files under `--container-system-data-folder` (default `/var/devcontainer/`)
so re-running set-up against the same container is a no-op.

## What this PR adds

- **New CLI flag**: `--container-system-data-folder <path>` (spec §6,
  default `/var/devcontainer`).
- **`apply_etc_patches`** wires both patches before the lifecycle hook
  phase (spec §5 puts system patches in phase 3a). Skipped when
  `--skip-post-create` is set, mirroring upstream's "post-create is the
  umbrella for user customization" semantics.
- **`build_etc_environment_patch_script`**: emits a shell script that
  appends a `# >>> deacon set-up >>>` ... `# <<< deacon set-up <<<`
  block of `KEY="VALUE"` lines to `/etc/environment`. Escapes `\` and
  `"` so the PAM parser doesn't choke. Touches the marker only after a
  successful append. Empty env → no-op (does NOT touch the marker, so a
  later run with a populated config still patches).
- **`build_etc_profile_patch_script`**: emits a shell script that
  re-exports PATH from `/etc/environment` inside a delimited block. This
  is the spec-§16 design-decision recipe — without it the env appended
  in `/etc/environment` doesn't reach login shells.
- **`collect_env_pairs`**: merges `containerEnv` + `remoteEnv` + CLI
  `--remote-env` overlays into a sorted, deterministic list. CLI wins on
  key conflicts. `None`-valued `remote_env` entries are dropped.
- **Best-effort**: any non-zero exit from the patch shell (no root,
  read-only `/etc`, missing `sh`, etc.) emits a WARN and proceeds. Spec §9:
  system-level patches "do not abort set-up unless critical".

## Tests

10 new unit tests:
- `build_etc_environment_patch_returns_noop_when_env_empty`
- `build_etc_environment_patch_short_circuits_when_marker_present`
- `build_etc_environment_patch_writes_sorted_env_block`
- `build_etc_environment_patch_wraps_block_in_delimiters`
- `build_etc_environment_patch_escapes_special_chars_in_values`
- `build_etc_profile_patch_short_circuits_on_marker`
- `build_etc_profile_patch_reexports_path_from_environment`
- `build_etc_profile_patch_wraps_in_delimiters`
- `collect_env_pairs_merges_config_remote_and_cli`
- `collect_env_pairs_returns_sorted_output`

Total set-up tests: 27 pass (14 from PR-6a + 3 from PR-6b + 10 new).

Verification:
- `cargo fmt --all -- --check`
- `cargo clippy --all-targets -- -D warnings`
- `cargo test -p deacon --lib` → 240 pass (no regression)

## With this PR, the set-up subcommand is functionally complete

The only spec-§5 item still on the deferred list is the live-container
`${containerEnv:VAR}` second-pass substitution — an enhancement, not a
behavior gap; the configured `container_env` is already substituted.

Refs: issue #34 (Tier 1 progress tracker); PR-6a (#36), PR-6b (#37).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pofallon pofallon mentioned this pull request May 25, 2026
Closed
pofallon added a commit that referenced this pull request May 25, 2026
@pofallon @claude
docs(changelog): add entries for PR-4b (#35) and set-up track (#36/#37/…
e6b929c 
…#38)

Issue #34's tracker explicitly flags that the CHANGELOG (currently on this
branch under [Unreleased]) should be extended to cover the four PRs opened
after PR-7 was first drafted:

- #35 — lockfile graduation phase 2 (writer wiring in up; closes #32)
- #36 — set-up subcommand MVP scaffold + lifecycle hooks
- #37 — set-up dotfiles installer
- #38 — set-up /etc/environment + /etc/profile root patches

All four are recorded under "Added" since they're net-new user-visible
surface. The existing "Changed"/"Fixed"/"Notes" sections are untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon added a commit that referenced this pull request May 25, 2026
@pofallon @claude
docs(changelog): add entries for PR-4b (#35) and set-up track (#36/#37/…
da85dab 
…#38)

Issue #34's tracker explicitly flags that the CHANGELOG (currently on this
branch under [Unreleased]) should be extended to cover the four PRs opened
after PR-7 was first drafted:

- #35 — lockfile graduation phase 2 (writer wiring in up; closes #32)
- #36 — set-up subcommand MVP scaffold + lifecycle hooks
- #37 — set-up dotfiles installer
- #38 — set-up /etc/environment + /etc/profile root patches

All four are recorded under "Added" since they're net-new user-visible
surface. The existing "Changed"/"Fixed"/"Notes" sections are untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pofallon added a commit that referenced this pull request May 25, 2026
@pofallon @claude
docs: Podman experimental status + 1.0 roadmap + CHANGELOG bootstrap …
d0c1d4d 
…(PR-7 of 7) (#31)

* docs: Podman experimental status, 1.0 roadmap, and CHANGELOG bootstrap

Last of the four parallelizable Tier 1 PRs from docs/ROADMAP_TO_1.0.md.
No runtime behavior changes beyond a single one-time WARN.

## Podman repositioned as experimental for 1.0

Trait-level Podman integration is already complete in crates/core/src/
runtime.rs (DockerRuntime + PodmanRuntime fully implement Docker,
ContainerOps, DockerLifecycle traits), but lacks test coverage and
rootless-Podman parity items (label=disable, --userns=keep-id,
--uidmap/--gidmap). README/CLAUDE.md previously said "planned" / "in
development" — that's not accurate, but neither is "fully supported".
Documenting as experimental for 1.0; promoting in 1.1 (tracked at #30).

- README.md "Currently Unsupported" row + "Runtime Selection" section:
  replace "Planned. Docker is fully supported; Podman coming later"
  with the experimental-status narrative + tracking issue link.
- CLAUDE.md "Container Runtimes" section: same update.
- crates/deacon/src/cli.rs: --runtime help text now reads "docker or
  podman [experimental]"; RuntimeOption::Podman doc comment marked.
- crates/core/src/runtime.rs: detect_runtime() emits a one-time WARN
  (std::sync::Once + tracing::warn!) when Podman is selected — by CLI
  flag or DEACON_RUNTIME env var. Idempotent across calls per process.

## CHANGELOG.md (new)

Keep-a-Changelog format, seeded with the Unreleased section capturing
PR-1..PR-3 + PR-7 changes. Includes a Notes subsection recording the
four "non-issues" surfaced by the 1.0 audit (serde_yaml not used; TAR
CVE not vulnerable; json5 advisory inapplicable; issue #1 already
resolved via bead 14b) so the audit trail lives in code, not memory.

## docs/ROADMAP_TO_1.0.md

The 1.0 readiness report produced ahead of the implementation work,
deferred from PR-1's chore commit per agreed scoping.

## Tracking issue filed

#30 — "Podman parity: promote from experimental to supported in 1.1"

## Verified

- cargo fmt --all -- --check
- cargo clippy --all-targets -- -D warnings
- cargo nextest run --profile dev-fast --no-default-features: 1927/1927 pass
- cargo test --doc --workspace: 130/130 pass
- 11/11 runtime::tests pass
- deacon --help shows updated runtime text and [experimental] marker

Refs: docs/ROADMAP_TO_1.0.md Tier 1 items "Podman story decision" and
"CHANGELOG.md".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(changelog): add entries for PR-4b (#35) and set-up track (#36/#37/#38)

Issue #34's tracker explicitly flags that the CHANGELOG (currently on this
branch under [Unreleased]) should be extended to cover the four PRs opened
after PR-7 was first drafted:

- #35 — lockfile graduation phase 2 (writer wiring in up; closes #32)
- #36 — set-up subcommand MVP scaffold + lifecycle hooks
- #37 — set-up dotfiles installer
- #38 — set-up /etc/environment + /etc/profile root patches

All four are recorded under "Added" since they're net-new user-visible
surface. The existing "Changed"/"Fixed"/"Notes" sections are untouched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@pofallon pofallon changed the base branch from pr6b-set-up-dotfiles to main May 25, 2026 19:01
pofallon added 2 commits May 25, 2026 19:10
@pofallon
Merge remote-tracking branch 'origin/main' into pr6c-set-up-etc-patches
ead92a3 
# Conflicts:
#	crates/deacon/src/cli.rs
#	crates/deacon/src/commands/set_up.rs
@pofallon
Merge remote-tracking branch 'origin/main' into pr6c-set-up-etc-patches
f62bba1 
# Conflicts:
#	crates/deacon/src/cli.rs
#	crates/deacon/src/commands/set_up.rs
@pofallon pofallon merged commit 6a56069 into main May 25, 2026
8 checks passed
@pofallon pofallon deleted the pr6c-set-up-etc-patches branch May 25, 2026 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pofallon