fix(deva): tighten mount dispatch and drop python path helpers#270
Merged
lroolle merged 10 commits intoMay 22, 2026
Conversation
Mount dispatch walked every CONFIG_ROOT subdir and emitted every loose
child as a bind mount. With ~/.config/deva/sessions/ holding 200+ files
this produced 200+ -v flags, and validate_bind_mount_shape's O(N^2) loop
with python3 forks per call turned a dry-run into a 2m42s stall.
- Narrow dispatch to known agent subdirs only (agents/<name>.sh gate)
- Mount only canonical entries per agent (.claude+.claude.json, .codex,
.gemini) instead of blind glob walk
- Delete dead should_mount_home_item / mount_loose_home_item /
mount_dir_contents_into_home — allowlist replaces denylist
- Default to hybrid mounts: all populated agent subdirs mount into every
container; --config-home DIR still isolates to a single home
- Replace python3 path helpers with pure bash (_normalize_path,
absolute_path, canonical_path, path_is_strict_descendant,
relative_subpath) — parity-tested against python across 30+ inputs
- Drop node/python probes from get_host_tmpdir; ${TMPDIR:-/tmp} suffices
- Add progressive --debug breadcrumbs (_step) at 9 phase boundaries
- Register claude-trace in TOOL_REGISTRY so version-upgrade can see it
- Extend test-mount-shape.sh: hybrid-default, --config-home isolation,
CLI -v override, zero-match count_target fix
Result: 228-file CONFIG_ROOT -> 4 mounts, 42ms wall-clock.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 5m 35s —— View job Security Audit: Mount Dispatch & Shell Injection Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 085378a8db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # ───── get_tools_by_group ───── | ||
| section "get_tools_by_group" | ||
| agent_tools="$(get_tools_by_group agent | sort | tr '\n' ' ' | sed 's/ $//')" | ||
| expected_agent="atlas-cli claude-code codex copilot-api gemini-cli" |
There was a problem hiding this comment.
The new claude-trace tool was added to TOOL_REGISTRY, but this test still expects the old tool lists, so bash tests/test_release_utils.sh now fails deterministically and the new CI unit-test step will fail on every run. The expected agent/main/default lists here need to include claude-trace to match the registry.
Useful? React with 👍 / 👎.
Comment on lines
426
to
428
| gemini-cli) env_var="GEMINI_CLI_VERSION" ;; | ||
| atlas-cli) env_var="ATLAS_CLI_VERSION" ;; | ||
| copilot-api) env_var="COPILOT_API_VERSION" ;; |
There was a problem hiding this comment.
This case does not handle claude-trace, so in load_versions the env_var value from the previous tool iteration is reused (currently COPILOT_API_VERSION). That makes claude-trace pick an unrelated version value in version-report/versions-up output and can cause false upgrade signals even when agent pins are actually current.
Useful? React with 👍 / 👎.
claude-trace's npm fetch garbles the version-report display. version-upgrade.sh pins it via $CLAUDE_TRACE_VERSION directly (line 88), so adding it to the registry only broke reporting without enabling auto-upgrade. Keep it pinned-only like playwright. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 3m 16s —— View job Security Audit: Mount Dispatch & Shell Injection Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
version-upgrade.sh pinned claude_trace_ver and playwright_ver to env vars instead of calling get_latest(). Also load_versions case statement missed claude-trace, so env_var kept the previous iteration's value (COPILOT_API_VERSION) — garbling the display with the copilot hash. - Add claude-trace to TOOL_REGISTRY with correct npm source - Add claude-trace case to load_versions env-var mapping - Add catch-all *) env_var="" to prevent future stale-variable bugs - Wire get_latest for claude-trace and playwright in version-upgrade.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 1m 59s —— View job Security Audit: Mount Dispatch & Shell Injection Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
load_versions used env vars as "latest", but versions.env always populates them via version-pins.sh — so the check compared pinned values against themselves and reported "up-to-date" unconditionally, even when npm had newer releases. - Remove env-var shortcut from load_versions; always fetch from upstream (npm/github) for the reporting path - Snapshot CLI overrides in version-upgrade.sh BEFORE version-pins.sh fills defaults, so explicit `CLAUDE_CODE_VERSION=X make versions-up` still forces that version at build time - Plain `make versions-up` now correctly detects and upgrades to the real latest upstream versions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 3m 21s —— View job Security Audit: Mount Dispatch & Shell Injection Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
- Rename display sections: "Pinned Agent Tools" -> "Agent Tools (auto-upgraded by make versions-up)", toolchains and playwright sections note "pinned — edit versions.env to bump" - Playwright stays pinned in version-upgrade.sh (uses $PLAYWRIGHT_VERSION not get_latest) — browser version bumps break test stability - Playwright still in TOOL_REGISTRY for `make versions` comparison, so you can see when a newer version exists without auto-upgrading Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 2m 29s —— View job Security Audit: Mount Dispatch & Path Handling Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
- Remove `npm install -g npm@latest` from copilot-api install step; npm replacing itself mid-execution corrupts its own node_modules when the cached layer has stale state - Remove the 5-attempt retry loop that only masked the real failure - Node 22 ships npm 10.x which handles `npm install -g pnpm` fine - Mount ~/.agents into container for agent state persistence Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 4m 7s —— View job Security Audit: Container & Authentication Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Required Fixes (Non-Security)🟡 P1 - Test Expectation Update Required:
🟡 P2 - Version Resolver Mapping:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
- Add deva.sh codex --browser-mcp: injects session-only Playwright MCP override via Codex --config, switches to rust image profile - Add CODEX_BROWSER_MCP=true and repeatable CODEX_CONFIG=key=value in .deva config files for persistent browser sessions - Bump Go 1.26.3, Claude Code 2.1.143, Codex 0.131.0, Gemini CLI 0.42.0, Playwright 1.60.0 - Redesign container slug: deva--<agent>--<auth_tag>--<slug>..<hash> replaces the old ..i/..v/..c suffix soup; agent always in name - Normalize short_hash to SHA-256; drop cksum fallback (non-hex) - Add scripts/test-codex-browser-shape.sh dry-run coverage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 3m 38s —— View job Security Audit: Container & Authentication Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Required Fixes (Non-Security)🟡 P1 - Test Expectation Update Required:
🟡 P2 - Version Resolver Mapping:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
USER_ENVS=("${retained[@]}") crashes under set -u when retained
is empty (all existing entries filtered out). Use explicit length
check to assign empty array safely.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Claude finished @lroolle's task in 2m 57s —— View job Security Audit: Container & Authentication Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Required Fixes (Non-Security)🟡 P1 - Test Expectation Update Required:
🟡 P2 - Version Resolver Mapping:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
Contributor
|
Claude finished @lroolle's task in 2m 43s —— View job Security Audit: Container & Authentication Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Required Fixes (Non-Security)🟡 P1 - Test Expectation Update Required:
🟡 P2 - Version Resolver Mapping:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
Contributor
|
Claude finished @lroolle's task in 3m 12s —— View job Security Audit: Mount Dispatch & Authentication Review ✅AUDIT COMPLETE: No blocking security issues found. Changes improve container security posture. Security Analysis Results🟢 MOUNT DISPATCH SECURITY - IMPROVED
🟢 BASH PATH HELPERS - SECURE IMPLEMENTATIONS
🟢 CONTAINER BOUNDARIES - MAINTAINED
🟢 AUTH & CONFIG ISOLATION - INTACT
🟢 INPUT SANITIZATION - ROBUST
Code Quality ObservationsPerformance Gains:
Defensive Programming:
TOOL_REGISTRY Security:
Required Fixes (Non-Security)🟡 P1 - Test Expectation Update Required:
🟡 P2 - Version Resolver Mapping:
Verdict: APPROVED FOR MERGE ✅This PR significantly improves security posture by:
No blocking security issues identified. Changes are security-positive. Required tests pass:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Stacked on #256 (tmux-bridge).