Open
Conversation
- Update status sections to reflect all phases complete - Fix CLI reference: remove non-existent commands/flags, add missing ones - Update ps output to show correct STATUS column (not HEALTH) - Mark restricted network mode as implemented (not planned) - Move API Proxy from "future" to implemented features in security.md - Add --git-worktree to workspace mode comparison in jj-workspaces.md - Remove non-existent --keep-skills flag reference - Update all example output to match actual CLI behavior Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Document identified issues and phased remediation plan: - Phase 1: Foundation (abstractions, SSH builder, validation) - Phase 2: Architecture improvements - Phase 3: Code quality - Phase 4: Testing infrastructure - Phase 5: Documentation
Phase 1.1 - Create interfaces for testability: - FileSystem: ReadFile, WriteFile, Remove, Stat, MkdirAll, etc. - CommandExecutor: Execute, ExecuteInteractive, ReplaceProcess - Default OS-based implementations - Mock implementations for testing with error injection
…lder Phase 1.2 - Eliminate SSH code duplication: - Add ssh.Options struct with fluent builder pattern - Add DefaultOptions(port) with sensible defaults - Add WithBatchMode(), WithTTY(), WithTimeout() methods - Update convenience functions to use builder internally - Update cmd/ssh.go, cmd/exec.go, runtime/nspawn.go to use builder
Phase 1.3 - Improve cleanup error handling: - cmd/up.go: Track and log cleanup failures, report error count - cmd/down.go: Log file removal errors (secrets, skills, config)
Phase 1.4 - Validate configs at load time: Go side: - Add Validate() to HostConfig, PortRange, Template, AgentConfig, SandboxMetadata - LoadHostConfig() and LoadTemplate() now validate after parsing - Update tests to include required fields in template fixtures NixOS module: - Add assertions for required user option - Add port range validation (from <= to, at least 10 ports) - Validate template secrets exist in secrets map - Update network mode description (restricted is now implemented)
Phase 2.2 - Add externalInterface option to NixOS module: - Add optional externalInterface option (null by default) - Skip NAT configuration when null (allows existing NAT setup) - Fixes hardcoded eth0 that breaks on many systems
Phase 2.3 - Unify Runtime Interface for SSH: - Add sandboxPorts map to NspawnRuntime to track ports - Store port during Create, return from SSHPort - Remove error-returning stub implementations
Phase 2.1 - Extract business logic from commands: - Create internal/sandbox package for sandbox lifecycle management - Move workspace setup, secret management, and health checks to Creator - Simplify cmd/up.go to orchestration only (~370 lines → ~100 lines) Also: - Add **/vendor/ to .gitignore to prevent accidental vendoring - Fix TestListTemplates to include required AuthEnvVar field - Update TECH_DEBT.md to mark Phase 2.1 as complete
Phase 3.1 - Define constants for magic values: - Add TmuxSessionName to config package - Add SSHReadyTimeoutSeconds to health package - Add NetworkSlotMin/NetworkSlotMax to port package - Add NixOSStateVersion to generator package - Update all hardcoded references to use these constants Phase 3.2 - Standardize error handling: - Add SandboxNotRunning, WorkspaceError, ValidationError constructors - Update cmd files to use consistent error types from errors package - Replace fmt.Errorf with typed errors for better exit codes Phase 3.3 - Consolidate logging: - Create internal/logging/user.go with UserInfo, UserSuccess, etc. - Update cmd/root.go to use aliases to logging package
Phase 4.1 - Add test fixtures: - Create internal/testutil/fixtures/ with JSON fixture files - Add fixtures.go with embed.FS-based fixture loading - Add fixtures_test.go to verify fixture loading - Add mock_test.go for MockFS and MockExecutor tests Phase 4.2 - Add unit tests for critical paths: - Add internal/ssh/ssh_test.go with SSH builder tests - Add internal/health/health_test.go with health check tests - All tests passing Phase 4.3 - Add integration test framework: - Create internal/integration/harness.go with TestHarness - Auto-skip when FORAGE_INTEGRATION_TESTS not set - Template, workspace, SSH wait helpers - Automatic cleanup via t.Cleanup
Phase 5.1 - Add package documentation: - Add doc.go files to all internal packages with godoc-compatible documentation explaining purpose, key types, usage examples - Packages documented: app, config, errors, gateway, generator, health, integration, logging, network, port, proxy, runtime, sandbox, skills, testutil, tui, workspace Phase 5.2 - Add Architecture Decision Records: - Create docs/adr/ directory with ADR index - ADR 001: Container Runtime Abstraction - ADR 002: SSH-Based Sandbox Access - ADR 003: Skill Injection Strategy - ADR 004: Workspace Modes (Direct, JJ, Git Worktree) ADRs complement DESIGN.md by documenting the 'why' behind architectural decisions, alternatives considered, and tradeoffs.
All technical debt remediation work is complete: - Phase 1: Foundation (abstractions, SSH builder, validation) - Phase 2: Architecture (business logic extraction, network config) - Phase 3: Code Quality (constants, error handling, logging) - Phase 4: Testing (fixtures, unit tests, integration harness) - Phase 5: Documentation (package docs, ADRs)
Set HOME to a writable temp directory and isolate git/jj config so tests pass in the nix build sandbox where HOME=/homeless-shelter.
readJJIdentity inherits the process HOME, so t.Setenv is needed in addition to the per-command sandboxEnv to ensure jj's secure-config repo tracking can access ~/.config/jj/repos/ during config get.
The ''${} prefix in Nix indented strings escapes interpolation,
producing literal ${pkgs.git} instead of resolving store paths.
Use plain ${...} for Nix interpolations (store paths) and reserve
''${} (via nixEscapeIndented) only for user-supplied values.
Also make jj config set non-fatal (|| true) so a jj failure doesn't
abort the script before git email and SSH config are written.
Add NixOS option `initCommands` (list of strings) to the template type and wire it into the JSON generation with `lib.optionalAttrs` so it's only included when non-empty. Add corresponding `InitCommands` field to the Go `Template` struct with `omitempty` for backward compatibility.
Add `InitCommandResult` struct to track template command execution counts, warnings, per-project init status, and add it as a field on `CreateResult`.
Add `runInitCommands` method that executes template-level init commands via `runtime.Exec()` with `sh -c`, then checks for and runs a per-project `.forage/init` script. Failed commands log warnings but do not block creation. Integrate as Phase 10 between SSH ready and audit log.
Add `displayInitResult()` helper to show init command counts, warnings, and per-project init status. Call it from both `runUp` and `createSandboxFromWizard` after sandbox creation.
Config tests: round-trip marshal/unmarshal, backward compatibility with old JSON format, omit-when-empty behavior. Creator tests: no-op when no commands, correct exec args (sh -c, user, workdir), failed commands continue execution, per-project .forage/init detection and execution.
Add Init Commands section to templates docs covering syntax, execution semantics, per-project .forage/init scripts, execution order, and a beads setup example. Update template structure overview to include initCommands.
Add composable workspace mount types to config: - WorkspaceMount: template-level mount spec with source, mode, branch - WorkspaceMountMeta: metadata record with resolved host paths - WorkspaceMounts field on Template (map keyed by name) - WorkspaceMounts field on SandboxMetadata (list) - Updated Validate() to accept either legacy or new workspace fields
Adds Repos map[string]string field to CreateOptions to support named repo references from --repo name=path CLI flags.
Add WorkspaceMountsContributor and ResolvedMount types to support composable workspace mounts. Keeps WorkspaceMountContributor for backward compatibility but marks it deprecated.
Add setupWorkspaceMounts() that iterates template WorkspaceMounts: - Resolves repo references (default, named, absolute) - Validates mount specs (duplicate paths, missing repos) - Creates VCS workspaces per-mount with rollback on failure - Managed paths under workspaces/<sandbox>/<mount-name>/ - Update createMetadata to populate WorkspaceMounts metadata - Create() dispatches to new or legacy path based on template
…ends Update buildContributionSources to use WorkspaceMountsContributor when WorkspaceMounts are present, falling back to legacy single-mount. Per-mount VCS backends are added as individual contributors. RebuildContainerConfig reconstructs mount backends from metadata.
Update Cleanup() to handle both new multi-mount metadata and legacy single-workspace metadata. Each VCS-backed mount is cleaned up individually, and the managed sandbox workspace subdirectory is removed.
- Change --repo from StringVarP to StringArrayVarP (repeatable) - Support name=path syntax for named repo parameters - Remove unconditional --repo required flag - Add clear error when --repo is missing and template has no mounts - Add parseRepoFlags to split default/named repos - Update parseCreateOptions to return error for invalid flags
Add composable workspace mount options to the NixOS module: - workspace.mounts: attrsOf mount specs with containerPath, hostPath, repo, mode, branch, readOnly - workspace.useBeads: convenience option that injects a jj mount at /workspace/.beads and adds the beads package to extraPackages - Template JSON generation includes workspaceMounts when configured - useBeads merges into workspace.mounts before JSON serialization
- Add Mounts field to system prompt template data - System prompt template shows mount list when composable mounts are used - Falls back to legacy single-workspace display when no mounts configured - VCS skill detection checks WorkspaceMounts for multi-mount metadata - Conventional commits skill checks multi-mount VCS modes
Add comprehensive tests for the new multi-mount functionality: - Config: WorkspaceMountMeta round-trip, validation, template JSON, backward compat with old metadata format - CLI: parseRepoFlags for default/named repo parsing - Creator: validateMountSpecs, resolveRepoPath, setupWorkspaceMounts for hostPath, missing hostPath, missing repo scenarios - Injection: WorkspaceMountsContributor mounts, read-only handling, empty mounts, legacy backward compat
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.
No description provided.