Skip to content

Support exact secondary-root Agent worktrees - #816

Open
nickdavis wants to merge 2 commits into
repoprompt:mainfrom
nickdavis:codex/fix-secondary-root-agent-worktrees
Open

Support exact secondary-root Agent worktrees#816
nickdavis wants to merge 2 commits into
repoprompt:mainfrom
nickdavis:codex/fix-secondary-root-agent-worktrees

Conversation

@nickdavis

Copy link
Copy Markdown
Contributor

Summary

  • authorize Agent worktree execution against the complete canonical saved-workspace root set, including exact worktrees for secondary repository roots
  • keep saved-workspace identity separate from session worktree projection while failing closed for stale, unmatched, duplicate, or ambiguous root bindings
  • preserve explicit secondary-repository selection through Agent start/explore paths and add direct create-session, bind, and builtin-review steer coverage

Validation

  • make dev-test FILTER=AgentRunWorktreeStartTests — 55 tests passed
  • make dev-lint
  • make dev-swift-build PRODUCT=RepoPrompt
  • .agents/skills/rpce-contribution-check/scripts/preflight.sh commit
  • .agents/skills/rpce-contribution-check/scripts/preflight.sh push
  • independent read-only review of origin/main..HEAD — no blockers

Additional validation context

The full local PR-ready root suite was attempted. It progressed through the affected coverage but did not complete: an unrelated durable-artifact concurrency test reported a busy publication result, and the run later stopped producing output in workspace-context tests. The focused Agent worktree suite, strict lint, and affected product build all passed.

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Audit disposition — direct correctness blocker (2026-08-14)

The complete-root authorization and fail-closed handling of stale, duplicate, and ambiguous bindings are good. The no-binding Context Builder branch still violates the intended physical-root contract, however: it returns workspaceDirectoryPath rather than the selected executionPath. A request targeting a stored/secondary physical root can therefore fall back to the active/primary workspace directory.

Return the selected canonical physical root when no session binding overrides it, add a focused stored-versus-active workspace regression, and rerun the failing app shard. Treat the separate Codex context-window failure as a baseline/rebase issue if it is unrelated. I would not merge until the root-selection regression is fixed.

morluto commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Deep-review assessment — 2026-08-14

Disposition: block on the no-binding execution-root regression. Authorizing bindings against the complete canonical workspace root set and rejecting stale/duplicate/ambiguous logical roots is the right model. However, ContextBuilderWorkspaceContext returns workspaceDirectoryPath when bindings is empty, even though the selected physical executionPath should remain authoritative. In a multi-root or inactive/stored selection this can route Context Builder back to the active/primary workspace.

Return the selected canonical physical execution root in the no-binding branch and add a regression where the selected secondary/stored workspace differs from the active primary workspace. This aligns with the reviewed exact-head shard failure that observed the active workspace instead of the stored selection. Rerun the failing shard and the full AgentRunWorktreeStartTests after the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants