refactor: Config.Sandbox struct + Context pipeline for stateful sessions#17
Merged
Conversation
Extends validate_path to accept a Config.Sandbox struct in addition to the existing string-based sandbox_root. The struct-based version tries each root until validation succeeds, using default_cwd for relative path resolution. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Split validate_path into three explicit clauses:
- /2 with %Config.Sandbox{} - preferred API
- /2 with string - backwards compat, no deprecation
- /3 with strings - now deprecated
Only the explicit 3-arg form shows deprecation warnings,
allowing gradual migration to the struct-based API.
- Add new build_config/0 function that returns %Config.Sandbox{}
- Deprecate build_context/0 (still works, but marked for removal)
- New struct has roots: [root] and default_cwd: root
- This completes Phase 1 of the TDD migration
TDD: wrote failing tests first, then implemented to pass.
…g.Sandbox struct Phase 2 migration: Update core execution pipeline to use struct-based API: - TrumanShell.execute: build_context() -> build_config() - Expander: accepts Config.Sandbox, with backward-compatible map support - Glob: accepts Config.Sandbox, with backward-compatible map support The backward-compatible map conversion allows incremental migration of remaining call sites without breaking existing tests.
- Replace deprecated validate_path/3 with validate_path/2 - Build SandboxConfig struct from sandbox_root and current_dir - Part of Phase 2 migration to struct-based API
Updated all remaining call sites in Phase 2 to use the new struct-based Sandbox.validate_path/2 API: - lib/truman_shell/commands/cd.ex - lib/truman_shell/commands/cp.ex - lib/truman_shell/commands/find.ex - lib/truman_shell/commands/grep.ex - lib/truman_shell/commands/ls.ex - lib/truman_shell/commands/mkdir.ex - lib/truman_shell/commands/mv.ex - lib/truman_shell/commands/rm.ex - lib/truman_shell/commands/touch.ex - lib/truman_shell/stages/redirector.ex - lib/truman_shell/support/file_io.ex Each file now converts context map to SandboxConfig struct using to_sandbox_config/1 helper. Uses sandbox_root as default_cwd since these modules pre-resolve paths relative to sandbox_root before calling validate_path. 546 tests passing.
- Remove deprecated `build_context/0` function (use `build_config/0` instead) - Remove deprecated `validate_path/3` function (use `validate_path/2` with Config.Sandbox) - Update tests to use struct-based API All 544 tests pass, credo clean, dialyzer clean. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Partial refactor in progress (DOES NOT COMPILE): - Renamed SandboxConfig fields: roots → allowed_paths, default_cwd → home_path - Created Commands.Context struct with current_path and sandbox_config - Updated Behaviour, Executor, Redirector to use new Context - Updated pwd.ex, cd.ex, ls.ex to new pattern Still TODO (files with old field names): - cp.ex, find.ex, grep.ex, mkdir.ex, mv.ex, rm.ex, touch.ex - file_io.ex, config.ex Design question to resolve: - Should path_allowed? accept relative paths? Or require callers to expand first?
- Rename fields: roots → allowed_paths, default_cwd → home_path - Simplify path_allowed? to require absolute paths (caller expands first) - Update file_io.ex to accept Context struct - Update all command files and tests to use new field names - Update test files to use Context struct instead of legacy map This completes the Config.Sandbox field rename started in the WIP commit. All 544 tests pass, credo clean, dialyzer clean. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- expander.ex: use home_path for tilde expansion instead of first allowed_path - glob.ex: check all allowed_paths for sandbox boundary, not just first This fixes the [root|_] pattern smell where we extracted just the first allowed_path instead of using the proper field or checking all paths. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Commands were crashing at runtime because Executor.build_context returns
%Context{} but commands were accessing legacy map fields (current_dir,
sandbox_root). Tests passed because they called commands directly with maps.
Updated all 7 affected commands to use Context struct:
- cp, mv, rm, mkdir, touch, find, grep
Changes:
- Use ctx.current_path instead of context.current_dir
- Use ctx.sandbox_config.home_path instead of context.sandbox_root
- Use ctx.sandbox_config directly instead of to_sandbox_config()
- Removed to_sandbox_config/1 helper from all commands
- Updated doctests to use Context struct
- Updated all corresponding test files
544 tests + 93 doctests pass.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When Context struct is provided, Expander now correctly: - Uses ctx.sandbox_config.home_path for tilde expansion (~) - Uses ctx.current_path for glob expansion base This fixes the semantic confusion where `cd subdir && ls *.txt` would incorrectly expand globs relative to home_path instead of the current working directory. Added new tests demonstrating the fix: - glob expands relative to current_path, not home_path - tilde still expands to home_path when current_path differs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Enforces the contract documented in the @doc: "Callers must expand relative paths before calling this function" Previously, relative paths were silently accepted and expanded against the process CWD, which could be outside the sandbox. Now raises ArgumentError with a helpful message. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
conroywhitney
added a commit
that referenced
this pull request
Jan 25, 2026
54aacb9 to
6e6921b
Compare
After `cd subdir`, `find .` and `grep -r . PATTERN` should search relative to current_path, not home_path. Previously these commands passed paths directly to validate_path without expanding first, causing them to resolve against home_path. TDD: Added tests verifying behavior when current_path != home_path. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Establishes proper Context pipeline architecture:
- TrumanShell.execute creates ctx once at the top
- Context flows through Expander → Executor → Commands
- Commands receive handle(args, ctx), use ctx, don't construct it
- Only cd modifies ctx by returning {:ok, "", ctx: new_ctx}
Key changes:
- Executor.run takes ctx instead of opts map
- Removed all Process.get/put global state from Executor
- Cd returns ctx: new_ctx instead of set_cwd side effect
- Glob.expand takes %Context{}, raises on invalid current_path
- All tests construct proper Context structs
Net reduction of 186 lines (removed complexity).
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… step
Commands no longer need DomePath.expand/relative_to ceremony.
Before: 3 lines (expand, relative_to, validate)
After: 1 line (validate_path with ctx)
- Added validate_path/2 clause for %Context{}
- Uses ctx.current_path for relative path expansion
- Simplified 9 commands: cd, cp, find, grep, ls, mkdir, mv, rm, touch
- Added 4 tests for Context behavior
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ctx is the single source of truth - don't destructure and pass pieces. Internal functions extract what they need from ctx. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update doctests in head.ex, tail.ex, wc.ex, cat.ex, date.ex, true.ex, false.ex, which.ex to construct proper Context with SandboxConfig - Update test files (head_test, tail_test, wc_test, cat_test) to use Context struct instead of old map format - Update redirector.ex to use Sandbox.validate_path(path, ctx) - Remove legacy map signature from file_io.ex (was unused) - Simplify file_io.read_file to use validate_path(path, ctx) directly Part of the Context flow cleanup - ctx created once at top, flows through. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Convert echo_test, true_test, false_test, date_test, which_test to use proper Context struct with build_ctx() helper - Consistent pattern across all command test files now Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update Tilde.expand/2 parameter: sandbox_root → home_path - Update rm.ex internal functions: sandbox_root → home_path - Update test file variables and build_ctx helpers to use home_path (executor_test, cd_test, ls_test, expander_test, glob_test) Aligns with Config.Sandbox.home_path terminology throughout codebase. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update moduledoc in expander.ex: ~ → home_path - Update comment in config.ex: ~ expands to home_path - Update sandbox.ex validate_path doc: sandbox_root → boundary - Rename validate_path string clause param: sandbox_root → boundary - Rename do_validate_path internal param to boundary The string clause is for tests; "boundary" is more semantically accurate. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Rename function in Support.Sandbox: sandbox_root → dome_root - Update usage in cli.ex - Update tests in sandbox_test.exs Aligns with TRUMAN_DOME env var naming. "Dome root" is clearer - it's specifically the root path from the dome configuration, not a generic "sandbox root". Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Replace TRUMAN_DOME env var approach with Config.discover(): - CLI now loads config from agents.yaml (or defaults) - TrumanShell.execute/1 uses Config.discover() + SandboxConfig.new() - Support.Sandbox is now purely a validation module - Remove ~170 lines of legacy code The sandbox boundaries and home_path now come from agents.yaml: - sandbox.roots → allowed_paths - sandbox.default_cwd → home_path This completes the Config.Sandbox migration from PR #15.
- Config struct now uses `sandbox: %Config.Sandbox{}` instead of
`roots`/`default_cwd` flat fields
- Config.Sandbox.from_yaml/1 parses YAML with proper field names:
`allowed_paths` and `home_path` (both required, never inferred)
- Context.from_config/1 builds runtime context from loaded config
- TrumanShell.execute accepts optional context, returns {ok, output, ctx}
- Executor.run returns {ok, output, ctx} for state management
- CLI updated for new 3-tuple return type
- Tests updated for new schema and return types
544 tests + 91 doctests passing, dialyzer clean.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
P0: CLI.validate/2 was using removed config.roots and config.default_cwd
fields - now correctly uses config.sandbox.allowed_paths and
config.sandbox.home_path
P1: Config.Sandbox documentation showed old YAML field names (roots,
default_cwd) but from_yaml/1 expects allowed_paths, home_path
Found by: LLM Review Round 04 (Gemini, GPT flagged blocker)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CLI Changes: - Remove env var support (TRUMAN_CMD, TRUMAN_VALIDATE_PATH, TRUMAN_CURRENT_DIR) - validate-path takes only path, uses config directly via Context.from_config - All config comes from agents.yaml only Context Changes: - Clear stdin in returned context to prevent leakage between commands Documentation/Tests: - Update all references from roots/default_cwd to allowed_paths/home_path - Add 5 integration tests for cd persistence across execute/2 calls - Tests cover: multi-cd, ls with context, relative paths, cd ~, stdin clearing 549 tests + 91 doctests passing Dialyzer clean Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- bin/truman-shell docs updated to match new CLI (no TRUMAN_DOME/current_dir) - CLI tests updated to work with Config defaults (sandbox = project root) - Removed test for current_dir argument (no longer supported) - Tests now use existing project files (lib/truman_shell.ex, README.md) 548 tests + 91 doctests passing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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
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
Complete refactor establishing proper separation between static config and dynamic runtime state, enabling stateful shell sessions where
cdpersists across commands.Architecture
Key Changes
Config.Sandbox (static, immutable)
allowed_paths- boundaries for validation (supports multiple roots + globs)home_path- wherecd ~goes, tilde expansion targetCommands.Context (dynamic runtime state)
current_path- changes withcd, used for relative path resolutionsandbox_config- reference to immutable Config.Sandboxstdin- cleared between commands to prevent leakageTrumanShell.execute/2
{:ok, output, ctx}for state managementcd lib→lsshows lib contentsCLI Simplified
agents.yaml(no env vars)validate-path <path>uses config directly via Contextexecute <cmd>,validate-path <path>,versionBreaking Changes
TrumanShell.execute/1now returns 3-tuple{:ok, output, ctx}allowed_paths/home_path(notroots/default_cwd)Test Plan
LLM Review Summary
🤖 Generated with Claude Code