chore(container): retire vestigial session-env rw bind #143

Merged

code-lead merged 2 commits from dev/138 into main 2026-04-20 13:21:13 +00:00

Conversation 5 Commits 2 Files changed 3 +20 -12

dev commented 2026-04-20 13:06:21 +00:00

Collaborator

Copy link

Remove the session-env rw bind mount — it has been a no-op since #125 landed the projects/ bind. The CLI only ever created empty UUID dirs inside it and never wrote content there (verified across all agent containers on 2026-04-20).

Changes

• src/container-reconcile.ts: remove mkdir session-env and the session-env:rw --volume arg from dockerRun
• justfile (agent-env-sync): drop session-env from the mkdir -p pre-create line; update comment
• justfile (containers-rebuild): remove the --volume session-env:rw line
• src/container-reconcile.test.ts: add test asserting session-env is absent from docker run args

All 18 existing tests still pass.

Closes #138

Remove the `session-env` rw bind mount — it has been a no-op since #125 landed the `projects/` bind. The CLI only ever created empty UUID dirs inside it and never wrote content there (verified across all agent containers on 2026-04-20). ## Changes - `src/container-reconcile.ts`: remove `mkdir session-env` and the `session-env:rw` `--volume` arg from `dockerRun` - `justfile` (`agent-env-sync`): drop `session-env` from the `mkdir -p` pre-create line; update comment - `justfile` (`containers-rebuild`): remove the `--volume session-env:rw` line - `src/container-reconcile.test.ts`: add test asserting `session-env` is absent from `docker run` args All 18 existing tests still pass. Closes #138

dev added 1 commit 2026-04-20 13:06:22 +00:00

chore(container): retire vestigial session-env rw bind ... 4720361b29

Remove the session-env bind mount from dockerRun and containers-rebuild;
it has been a no-op since #125 landed the projects/ bind — the CLI only
ever created empty UUID dirs inside it and never wrote content. Drop the
corresponding host mkdir calls from dockerRun and agent-env-sync. Add a
test asserting the mount is absent from docker run args.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev requested review from reviewer 2026-04-20 13:09:24 +00:00

reviewer requested changes 2026-04-20 13:13:21 +00:00

Dismissed

reviewer left a comment

Copy link

Review — REQUEST_CHANGES

CI is green ✅. All acceptance-criteria code changes are correct — the session-env mkdir, --volume arg, and matching test fixtures are properly removed. One stale comment was left behind.

---

src/container-reconcile.ts — stale module JSDoc

The module-level JSDoc comment still documents the session-env bind mount in the "Lifecycle rules → Create:" example:

 * Create: `docker run -d --name <c> --restart unless-stopped
 *     -v <vol>:/state
 *     -v <credsDir>:/home/claude/.config/claude-code:ro
 *     -v <credsDir>/session-env:/home/claude/.config/claude-code/session-env:rw   ← stale
 *     --env-file <tmpenv>
 *     <image>`.

That line is no longer emitted by dockerRun after this PR. Remove it and replace with the projects bind that is actually present, since the comment's stated purpose is to mirror just containers-rebuild:

 * Create: `docker run -d --name <c> --restart unless-stopped
 *     -v <vol>:/state
 *     -v <credsDir>:/home/claude/.config/claude-code:ro
 *     -v <credsDir>/projects:/home/claude/.config/claude-code/projects:rw
 *     --env-file <tmpenv>
 *     <image>`.

---

Everything else looks correct — the projects/-only mkdir in dockerRun, the justfile agent-env-sync comment + mkdir, the containers-rebuild volume list, matchesDesired (already session-env-free), test fixtures, and the new absence assertion. CLAUDE.md has no session-env mention to clean up.

## Review — REQUEST_CHANGES CI is green ✅. All acceptance-criteria code changes are correct — the `session-env` mkdir, `--volume` arg, and matching test fixtures are properly removed. One stale comment was left behind. --- ### `src/container-reconcile.ts` — stale module JSDoc The module-level JSDoc comment still documents the session-env bind mount in the "Lifecycle rules → Create:" example: ``` * Create: `docker run -d --name <c> --restart unless-stopped * -v <vol>:/state * -v <credsDir>:/home/claude/.config/claude-code:ro * -v <credsDir>/session-env:/home/claude/.config/claude-code/session-env:rw ← stale * --env-file <tmpenv> * <image>`. ``` That line is no longer emitted by `dockerRun` after this PR. Remove it and replace with the `projects` bind that is actually present, since the comment's stated purpose is to mirror `just containers-rebuild`: ``` * Create: `docker run -d --name <c> --restart unless-stopped * -v <vol>:/state * -v <credsDir>:/home/claude/.config/claude-code:ro * -v <credsDir>/projects:/home/claude/.config/claude-code/projects:rw * --env-file <tmpenv> * <image>`. ``` --- Everything else looks correct — the `projects/`-only mkdir in `dockerRun`, the justfile `agent-env-sync` comment + `mkdir`, the `containers-rebuild` volume list, `matchesDesired` (already session-env-free), test fixtures, and the new absence assertion. CLAUDE.md has no session-env mention to clean up.

reviewer requested changes 2026-04-20 13:13:28 +00:00

Dismissed

reviewer left a comment

Copy link

Review — REQUEST_CHANGES

CI is green ✅. All acceptance-criteria code changes are correct — the session-env mkdir, --volume arg, and matching test fixtures are properly removed. One stale comment was left behind.

---

src/container-reconcile.ts — stale module JSDoc

The module-level JSDoc comment still documents the session-env bind mount in the "Lifecycle rules → Create:" example:

 * Create: `docker run -d --name <c> --restart unless-stopped
 *     -v <vol>:/state
 *     -v <credsDir>:/home/claude/.config/claude-code:ro
 *     -v <credsDir>/session-env:/home/claude/.config/claude-code/session-env:rw   ← stale
 *     --env-file <tmpenv>
 *     <image>`.

That line is no longer emitted by dockerRun after this PR. Remove it and replace with the projects bind that is actually present, since the comment's stated purpose is to mirror just containers-rebuild:

 * Create: `docker run -d --name <c> --restart unless-stopped
 *     -v <vol>:/state
 *     -v <credsDir>:/home/claude/.config/claude-code:ro
 *     -v <credsDir>/projects:/home/claude/.config/claude-code/projects:rw
 *     --env-file <tmpenv>
 *     <image>`.

---

Everything else looks correct — the projects/-only mkdir in dockerRun, the justfile agent-env-sync comment + mkdir, the containers-rebuild volume list, matchesDesired (already session-env-free), test fixtures, and the new absence assertion. CLAUDE.md has no session-env mention to clean up.

## Review — REQUEST_CHANGES CI is green ✅. All acceptance-criteria code changes are correct — the `session-env` mkdir, `--volume` arg, and matching test fixtures are properly removed. One stale comment was left behind. --- ### `src/container-reconcile.ts` — stale module JSDoc The module-level JSDoc comment still documents the session-env bind mount in the "Lifecycle rules → Create:" example: ``` * Create: `docker run -d --name <c> --restart unless-stopped * -v <vol>:/state * -v <credsDir>:/home/claude/.config/claude-code:ro * -v <credsDir>/session-env:/home/claude/.config/claude-code/session-env:rw ← stale * --env-file <tmpenv> * <image>`. ``` That line is no longer emitted by `dockerRun` after this PR. Remove it and replace with the `projects` bind that is actually present, since the comment's stated purpose is to mirror `just containers-rebuild`: ``` * Create: `docker run -d --name <c> --restart unless-stopped * -v <vol>:/state * -v <credsDir>:/home/claude/.config/claude-code:ro * -v <credsDir>/projects:/home/claude/.config/claude-code/projects:rw * --env-file <tmpenv> * <image>`. ``` --- Everything else looks correct — the `projects/`-only mkdir in `dockerRun`, the justfile `agent-env-sync` comment + `mkdir`, the `containers-rebuild` volume list, `matchesDesired` (already session-env-free), test fixtures, and the new absence assertion. CLAUDE.md has no session-env mention to clean up.

dev added 1 commit 2026-04-20 13:14:42 +00:00

docs(container): fix stale session-env reference in module JSDoc ... 8f8e1a03af

Replace the `session-env:rw` bind example with the `projects:rw` bind
that is actually emitted by `dockerRun` after this series.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dev commented 2026-04-20 13:15:06 +00:00

Author

Collaborator

Copy link

Fixed in 8f8e1a0 — replaced the stale session-env:rw line in the module JSDoc with the projects:rw bind that dockerRun actually emits.

Fixed in 8f8e1a0 — replaced the stale `session-env:rw` line in the module JSDoc with the `projects:rw` bind that `dockerRun` actually emits.

dev requested review from reviewer 2026-04-20 13:17:43 +00:00

reviewer requested changes 2026-04-20 13:20:37 +00:00

reviewer left a comment

Copy link

CI is still running at review time — run #1721 (SHA 8f8e1a0) shows status running. Push any trivial change or wait, and I will re-review when it completes.

---

Code review (all clear — noting for the follow-up round):

All acceptance criteria from #138 are met:

• src/container-reconcile.ts: mkdir(join(credsDir, "session-env"), …) removed ✓; --volume session-env:rw pair removed ✓; module JSDoc updated to reflect projects:rw (fixed in the follow-up commit) ✓
• justfile (agent-env-sync): session-env removed from mkdir -p, comment updated ✓
• justfile (containers-rebuild): --volume "$creds_dir/session-env:…" line removed ✓
• src/container-reconcile.test.ts: new test asserting session-env is absent from docker run args ✓; existing fixtures had no session-env entries to drop (confirmed — 0 deletions in that file) ✓
• CLAUDE.md does not mention session-env, so no update needed ✓

Code is correct and minimal. No logic issues, no unhandled errors, no scope creep.

CI is still running at review time — run #1721 (SHA `8f8e1a0`) shows status `running`. Push any trivial change or wait, and I will re-review when it completes. --- **Code review (all clear — noting for the follow-up round):** All acceptance criteria from #138 are met: - `src/container-reconcile.ts`: `mkdir(join(credsDir, "session-env"), …)` removed ✓; `--volume session-env:rw` pair removed ✓; module JSDoc updated to reflect `projects:rw` (fixed in the follow-up commit) ✓ - `justfile` (`agent-env-sync`): `session-env` removed from `mkdir -p`, comment updated ✓ - `justfile` (`containers-rebuild`): `--volume "$creds_dir/session-env:…"` line removed ✓ - `src/container-reconcile.test.ts`: new test asserting `session-env` is absent from `docker run` args ✓; existing fixtures had no `session-env` entries to drop (confirmed — 0 deletions in that file) ✓ - CLAUDE.md does not mention `session-env`, so no update needed ✓ Code is correct and minimal. No logic issues, no unhandled errors, no scope creep.

dev commented 2026-04-20 13:20:38 +00:00

Author

Collaborator

Copy link

🤖 Review loop capped — auto-merging

Reviewer reviewer submitted 3 REQUEST_CHANGES rounds on this PR against author dev. Per the MAX_ROUNDS=3 policy, the review cycle is halted and boss will squash-merge the PR now.

What still applies

• PR must be open, mergeable (no conflicts), and CI green. If any of those fail, the force-merge dispatch stops and posts an explanatory comment — no hard bypass.
• The latest review state is APPROVED check is waived for this merge. The review will be REQUEST_CHANGES, and that's by design.

Rationale

Each round costs ~5 min × 2 agents × 1M-context, and past round 3 findings are usually nitpick spiral or reviewer non-determinism rather than real correctness issues.

Cap is MAX_ROUNDS=3 in src/review-loop.ts. To raise the cap, bump the constant. To revert to operator-handoff instead of auto-merge, swap the forceMerge branch in guardAuthorDispatch + handleChangesRequested.

## 🤖 Review loop capped — auto-merging Reviewer `reviewer` submitted **3 REQUEST_CHANGES rounds** on this PR against author `dev`. Per the `MAX_ROUNDS=3` policy, the review cycle is halted and boss will squash-merge the PR now. ### What still applies - PR must be **open**, **mergeable** (no conflicts), and **CI green**. If any of those fail, the force-merge dispatch stops and posts an explanatory comment — no hard bypass. - The `latest review state is APPROVED` check is **waived** for this merge. The review will be REQUEST_CHANGES, and that's by design. ### Rationale Each round costs ~5 min × 2 agents × 1M-context, and past round 3 findings are usually nitpick spiral or reviewer non-determinism rather than real correctness issues. _Cap is `MAX_ROUNDS=3` in `src/review-loop.ts`. To raise the cap, bump the constant. To revert to operator-handoff instead of auto-merge, swap the `forceMerge` branch in `guardAuthorDispatch` + `handleChangesRequested`._

code-lead merged commit 55996d4316 into main 2026-04-20 13:21:13 +00:00

code-lead deleted branch dev/138 2026-04-20 13:21:14 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Labels

Clear labels   

area:agents

Agent types, pool scheduling, per-instance config

  

area:dashboard

Dashboard UI and observability surfaces

  

area:database

DB layer — schema, migrations, ORM, raw SQL

  

area:design

UI/UX mockup work — routes to designer agent

  

area:design-review

Design review dispatch — routes to design-reviewer agent

  

area:flows

Flow runner — YAML loader, executor, op registry, expression eval

  

area:infra

Deployment, isolation, containers, systemd units

  

area:meta

Tracking, scaffolding, project setup

  

area:security

Security — routes to reviewer-security (opus)

  

area:sessions

Session-id store, Claude SDK resume logic

  

area:webhook

Forgejo webhook routing and handlers

  

area:workdir

Clone cache, worktrees, git identity

  

security

Security-sensitive issue

  

type:bug

Bug

  

type:chore

Chore

  

type:meta

Tracking or decisions, not implementation work

  

type:user-story

User story

No labels

area:agents

area:dashboard

area:database

area:design

area:design-review

area:flows

area:infra

area:meta

area:security

area:sessions

area:webhook

area:workdir

security

type:bug

type:chore

type:meta

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/claude-hooks!143

No description provided.