Skip to content

Commit 7f3a36c

Browse files
fix: address Copilot review (PR #168 round 1)
9 review comments, all addressed: entrypoint.sh: - /workspace/CLAUDE.md is now chmod 600 (was 644) — orchestrators may embed credentials or private guidance; matches the mode used for ~/.claude/settings.json and ~/.git-credentials earlier in the script. - Plugin copy is now idempotent across re-runs against a persistent /workspace volume. Without the rm-first the 'cp -a src dst' pattern against an existing dst/ creates a nested dst/<basename>/ tree. tests/integration/test_entrypoint_workspace_injection.py: - Removed unused 'json' and 'tempfile' imports. lib/python/agentic_isolation/agentic_isolation/workspace_files.py: - inject() now validates container_path is absolute and has a non-empty basename, raising ValueError otherwise. Was silently producing tar entries with empty/invalid filenames for paths like '/' or 'relative/path'. - Two new unit tests cover the rejection paths. docs/workspace.md: - Fixed Python snippet — was mixing docker_client + client variable names; copy-paste-runnable now. docs/superpowers/plans/2026-05-12-workspace-injection-contract.md: - Replaced the two 'docker build providers/workspaces/claude-cli' invocations with the canonical 'just build-workspace-claude-cli' (docs/issues/002 had already noted this). CLAUDE.md: - Removed the absolute /Users/neural/... path; replaced with a link to the sibling repo's Gitea URL. docs/handoff-workspace-files-primitive.md: - Deleted. The original handoff doc that kicked off this brainstorming described the OLD per-domain contract (/etc/agentic/domain, AGENTIC_DOMAIN_*, AGENTIC_ALLOWED_TOOLS, entrypoint preamble templating). The merged spec + ADR-035 + docs/workspace.md supersede it. Git history preserves it. 176 Python tests + 7 integration tests + 1 OpenAPI snapshot all green.
1 parent 0f1f9ef commit 7f3a36c

8 files changed

Lines changed: 63 additions & 228 deletions

File tree

CLAUDE.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ Atomic building blocks for AI agent systems. Claude Code plugins + Python librar
1515
> at [`providers/workspaces/claude-cli/scripts/entrypoint.sh`](providers/workspaces/claude-cli/scripts/entrypoint.sh)
1616
> (section 5.5) is the source of truth for behavior;
1717
> [ADR-035](docs/adrs/035-workspace-injection-contract.md) is the decision
18-
> record. The sibling consumer (the agentic-domain-runner) is at
19-
> `/Users/neural/Code/HomeLab/agentic-domain-runner`.
18+
> record. The reference consumer is the
19+
> [agentic-domain-runner](https://gitea.neuralempowerment.xyz/HomeLab/agentic-domain-runner)
20+
> (a sibling repo).
2021
2122
## Repo Structure
2223

docs/handoff-workspace-files-primitive.md

Lines changed: 0 additions & 217 deletions
This file was deleted.

docs/superpowers/plans/2026-05-12-workspace-injection-contract.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,10 @@ fi
460460
- [ ] **Step 3: Rebuild the workspace image**
461461

462462
```bash
463-
docker build -t agentic-workspace-claude-cli:latest providers/workspaces/claude-cli
463+
# Canonical build (uses scripts/build-provider.py under the hood — stages
464+
# the build context the Dockerfile expects). Raw `docker build` against
465+
# providers/workspaces/claude-cli/ does NOT work; see docs/issues/002.
466+
just build-workspace-claude-cli
464467
```
465468

466469
Expected: clean build, no shellcheck warnings on the new section.
@@ -1343,11 +1346,14 @@ Pick the next tag (e.g., if `latest` aliases `0.7.x`, tag `0.8.0`). Conventions
13431346

13441347
```bash
13451348
cd /Users/neural/Code/AgentParadise/agentic-primitives
1346-
docker build -t agentic-workspace-claude-cli:0.8.0 \
1347-
-t agentic-workspace-claude-cli:latest \
1348-
providers/workspaces/claude-cli
1349+
just build-workspace-claude-cli # tags `latest` + the bundled Claude CLI version
1350+
# (or: uv run scripts/build-provider.py claude-cli)
13491351
```
13501352

1353+
`docker build -t ... providers/workspaces/claude-cli` does NOT work
1354+
— the Dockerfile expects a staged build context that the script
1355+
above produces. See docs/issues/002.
1356+
13511357
- [ ] **Step 3: (Optional) Push to the registry**
13521358

13531359
If the project uses GHCR or a private registry, push both tags. Otherwise leave at local only.

docs/workspace.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ mount + env:
107107
```python
108108
from agentic_isolation import WorkspaceFiles
109109

110-
wf = WorkspaceFiles(client=docker_client)
110+
client = docker_client
111+
wf = WorkspaceFiles(client=client)
111112

112113
# Bind-mount mode (host-resident content)
113114
mount = wf.bind_mount(workspace_dir, "/etc/agentic/workspace", read_only=True)

lib/python/agentic_isolation/agentic_isolation/workspace_files.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,21 @@ def inject(
6969
7070
Must be called after ``containers.create()`` and before
7171
``container.start()`` — the put_archive API requires the container
72-
to exist but works regardless of running state.
72+
to exist but works regardless of running state. The
73+
``container_path``'s parent directory must already exist inside
74+
the container; ``put_archive`` does not create intermediate
75+
directories.
76+
77+
Raises ``ValueError`` if ``container_path`` is not an absolute
78+
path or has an empty basename (e.g. ``/`` or trailing slash).
7379
"""
7480
target = Path(container_path)
81+
if not target.is_absolute():
82+
raise ValueError(f"container_path must be absolute, got {container_path!r}")
83+
if not target.name:
84+
raise ValueError(
85+
f"container_path must have a non-empty basename, got {container_path!r}"
86+
)
7587
parent = str(target.parent)
7688
basename = target.name
7789

lib/python/agentic_isolation/tests/test_workspace_files.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,29 @@ def test_inject_archives_and_calls_put_archive():
7070
# Quick sanity: tar archives start with the filename bytes in the
7171
# header at offset 0 — the basename should appear in the first 100 bytes.
7272
assert b"CLAUDE.md" in archive_bytes[:200]
73+
74+
75+
def test_inject_rejects_relative_path():
76+
"""inject() raises ValueError on non-absolute container_path."""
77+
from agentic_isolation.workspace_files import WorkspaceFiles
78+
79+
wf = WorkspaceFiles(client=MagicMock())
80+
try:
81+
wf.inject("ctr", "relative/path", b"x")
82+
except ValueError as e:
83+
assert "absolute" in str(e)
84+
else:
85+
raise AssertionError("expected ValueError")
86+
87+
88+
def test_inject_rejects_empty_basename():
89+
"""inject() raises ValueError on a path whose .name is empty (e.g. /)."""
90+
from agentic_isolation.workspace_files import WorkspaceFiles
91+
92+
wf = WorkspaceFiles(client=MagicMock())
93+
try:
94+
wf.inject("ctr", "/", b"x")
95+
except ValueError as e:
96+
assert "basename" in str(e)
97+
else:
98+
raise AssertionError("expected ValueError")

providers/workspaces/claude-cli/scripts/entrypoint.sh

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,10 @@ if [ -d "${INJECT_MOUNT}" ]; then
234234
ctx_src="${INJECT_MOUNT}/${AGENTIC_WORKSPACE_CONTEXT:-${INJECT_DEFAULT_CONTEXT}}"
235235
if [ -f "${ctx_src}" ]; then
236236
cp "${ctx_src}" "${INJECT_TARGET_CONTEXT}"
237-
chmod 644 "${INJECT_TARGET_CONTEXT}"
237+
# 600 because orchestrators may embed credentials or
238+
# private guidance in the workspace context. Matches the mode
239+
# used for ~/.claude/settings.json and ~/.git-credentials above.
240+
chmod 600 "${INJECT_TARGET_CONTEXT}"
238241
fi
239242

240243
if [ -d "${INJECT_MOUNT_PLUGINS}" ]; then
@@ -243,6 +246,11 @@ if [ -d "${INJECT_MOUNT}" ]; then
243246
[ -n "${plugin}" ] || continue
244247
src="${INJECT_MOUNT_PLUGINS}/${plugin}"
245248
[ -f "${src}/${INJECT_PLUGIN_MANIFEST}" ] || continue
249+
# rm first → idempotent across re-runs when /workspace is a
250+
# persistent named volume. Without the rm, `cp -a src dst`
251+
# against an existing dst/ creates a nested dst/<basename>/
252+
# tree instead of overwriting.
253+
rm -rf "${INJECT_TARGET_PLUGINS}/${plugin}"
246254
cp -a "${src}" "${INJECT_TARGET_PLUGINS}/${plugin}"
247255
AGENTIC_PLUGIN_FLAGS="${AGENTIC_PLUGIN_FLAGS} --plugin-dir ${INJECT_TARGET_PLUGINS}/${plugin}"
248256
done < <(__inject_names "${AGENTIC_WORKSPACE_PLUGINS:-}" "${INJECT_MOUNT_PLUGINS}")

tests/integration/test_entrypoint_workspace_injection.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88
See spec: docs/superpowers/specs/2026-05-12-workspace-injection-contract-design.md
99
"""
1010

11-
import json
1211
import subprocess
13-
import tempfile
1412
from pathlib import Path
1513

1614
import pytest

0 commit comments

Comments
 (0)