From 0d4f5971ea4336e26bb12fa1b81b33177e0c94fa Mon Sep 17 00:00:00 2001 From: Ricardo Decal Date: Sat, 28 Feb 2026 21:37:07 -0800 Subject: [PATCH] fix: resolve repo_access through sandbox profile merge cascade Multiple call sites were reading repo_access directly from the raw per-sandbox WorkspaceConfig instead of the three-tier merged config (universal < profile < per-sandbox). This caused sandboxes that inherit repo_access from a sandbox profile (e.g. admin-1 via pynchy-dev) to get repo_access=None, resulting in: - No /workspace/project mount (6 mounts instead of 9) - Container crash: "Working directory does not exist: /workspace/project" - Infinite retry loop on admin-1 Updated all call sites to use load_resolved_config() which correctly resolves repo_access through the merge cascade. --- .../host/container_manager/credentials.py | 8 +++-- src/pynchy/host/git_ops/repo.py | 13 +++++---- src/pynchy/host/orchestrator/lifecycle.py | 9 ++++-- .../host/orchestrator/workspace_config.py | 29 ++++++++++--------- tests/test_repo_tokens.py | 15 ++++++++++ 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/src/pynchy/host/container_manager/credentials.py b/src/pynchy/host/container_manager/credentials.py index 9f25fb9..550b6a5 100644 --- a/src/pynchy/host/container_manager/credentials.py +++ b/src/pynchy/host/container_manager/credentials.py @@ -161,9 +161,11 @@ def _write_env_file(*, is_admin: bool, group_folder: str) -> Path | None: logger.debug("Using GitHub token from gh CLI") else: # Non-admin: inject repo-scoped token if this workspace has repo_access - ws_cfg = s.workspaces.get(group_folder) - if ws_cfg and ws_cfg.repo_access: - repo_cfg = s.repos.get(ws_cfg.repo_access) + from pynchy.host.orchestrator.workspace_config import load_resolved_config + + resolved = load_resolved_config(group_folder) + if resolved and resolved.repo_access: + repo_cfg = s.repos.get(resolved.repo_access) if repo_cfg and repo_cfg.token: env_vars["GH_TOKEN"] = repo_cfg.token.get_secret_value() diff --git a/src/pynchy/host/git_ops/repo.py b/src/pynchy/host/git_ops/repo.py index 8263562..e17f6ac 100644 --- a/src/pynchy/host/git_ops/repo.py +++ b/src/pynchy/host/git_ops/repo.py @@ -136,17 +136,18 @@ def ensure_repo_cloned(repo_ctx: RepoContext) -> bool: def resolve_repo_for_group(group_folder: str) -> RepoContext | None: - """Look up workspace config.repo_access and return the resolved RepoContext. + """Look up the merged sandbox config's repo_access and return the resolved RepoContext. + Uses the three-tier merge cascade (universal < profile < per-sandbox) so that + ``repo_access`` inherited from a sandbox profile is correctly resolved. Returns None if the group has no repo_access or the slug is not configured. """ - from pynchy.config import get_settings + from pynchy.host.orchestrator.workspace_config import load_resolved_config - s = get_settings() - ws_cfg = s.workspaces.get(group_folder) - if ws_cfg is None or not ws_cfg.repo_access: + resolved = load_resolved_config(group_folder) + if resolved is None or not resolved.repo_access: return None - return get_repo_context(ws_cfg.repo_access) + return get_repo_context(resolved.repo_access) def check_token_expiry(slug: str, token: str) -> None: diff --git a/src/pynchy/host/orchestrator/lifecycle.py b/src/pynchy/host/orchestrator/lifecycle.py index 4454235..8ec0c38 100644 --- a/src/pynchy/host/orchestrator/lifecycle.py +++ b/src/pynchy/host/orchestrator/lifecycle.py @@ -178,10 +178,13 @@ async def _reconcile_state(app: PynchyApp) -> dict[str, list[str]]: s = get_settings() + from pynchy.host.orchestrator.workspace_config import load_resolved_config + repo_groups: dict[str, list[str]] = {} - for folder, ws_cfg in s.workspaces.items(): - if ws_cfg.repo_access: - repo_groups.setdefault(ws_cfg.repo_access, []).append(folder) + for folder in s.workspaces: + resolved = load_resolved_config(folder) + if resolved and resolved.repo_access: + repo_groups.setdefault(resolved.repo_access, []).append(folder) await asyncio.to_thread( reconcile_worktrees_at_startup, diff --git a/src/pynchy/host/orchestrator/workspace_config.py b/src/pynchy/host/orchestrator/workspace_config.py index f958c7e..5de6f9a 100644 --- a/src/pynchy/host/orchestrator/workspace_config.py +++ b/src/pynchy/host/orchestrator/workspace_config.py @@ -196,11 +196,11 @@ def load_resolved_config(group_folder: str) -> ResolvedSandboxConfig | None: def get_repo_access(group: WorkspaceProfile) -> str | None: """Return the repo_access slug for a group, or None if not configured. - Unlike the old has_pynchy_repo_access, admin groups no longer get implicit - access — they must set repo_access explicitly in config.toml. + Uses the three-tier merge cascade so that repo_access inherited from a + sandbox profile is correctly resolved. """ - config = load_workspace_config(group.folder) - slug = config.repo_access if config else None + resolved = load_resolved_config(group.folder) + slug = resolved.repo_access if resolved else None logger.debug( "Checked repo access", folder=group.folder, @@ -212,13 +212,14 @@ def get_repo_access(group: WorkspaceProfile) -> str | None: def get_repo_access_groups(workspaces: dict[str, Any]) -> dict[str, list[str]]: """Return a mapping of slug → list of group folder names with repo_access. - Only groups with an explicit repo_access slug in config.toml are included. + Uses the three-tier merge cascade so that repo_access inherited from a + sandbox profile is correctly resolved. """ result: dict[str, list[str]] = {} for profile in workspaces.values(): - config = load_workspace_config(profile.folder) - if config and config.repo_access: - result.setdefault(config.repo_access, []).append(profile.folder) + resolved = load_resolved_config(profile.folder) + if resolved and resolved.repo_access: + result.setdefault(resolved.repo_access, []).append(profile.folder) return result @@ -243,13 +244,15 @@ async def reconcile_workspaces( reconciled = 0 for folder, spec in specs.items(): config = spec.config + resolved = load_resolved_config(folder) context_mode = config.context_mode or s.sandbox_universal.context_mode or "group" + resolved_repo_access = resolved.repo_access if resolved else config.repo_access if config.name: display_name = config.name - elif config.repo_access: + elif resolved_repo_access: # Slack channel names can't contain slashes — use double-dash convention - display_name = config.repo_access.replace("/", "--") + display_name = resolved_repo_access.replace("/", "--") else: display_name = folder.replace("-", " ").title() @@ -339,7 +342,7 @@ async def reconcile_workspaces( "schedule_type": "cron", "schedule_value": config.schedule, "context_mode": context_mode, - "repo_access": config.repo_access, + "repo_access": resolved_repo_access, "next_run": next_run, "status": "active", "created_at": datetime.now(UTC).isoformat(), @@ -358,8 +361,8 @@ async def reconcile_workspaces( updates["next_run"] = compute_next_run("cron", config.schedule, s.timezone) if existing_task.prompt != config.prompt: updates["prompt"] = config.prompt - if existing_task.repo_access != config.repo_access: - updates["repo_access"] = config.repo_access + if existing_task.repo_access != resolved_repo_access: + updates["repo_access"] = resolved_repo_access if updates: await update_task(existing_task.id, updates) logger.info( diff --git a/tests/test_repo_tokens.py b/tests/test_repo_tokens.py index 2698da7..c39cde5 100644 --- a/tests/test_repo_tokens.py +++ b/tests/test_repo_tokens.py @@ -279,6 +279,7 @@ def test_non_admin_with_repo_access_gets_scoped_token(self, tmp_path: Path): }, secrets=MagicMock(gh_token=SecretStr(BROAD_TOKEN)), ) + fake_resolved = MagicMock(repo_access=REPO_SLUG) with ( patch("pynchy.host.container_manager.credentials.get_settings", return_value=s), patch("pynchy.host.container_manager.gateway.get_gateway", return_value=None), @@ -286,6 +287,10 @@ def test_non_admin_with_repo_access_gets_scoped_token(self, tmp_path: Path): "pynchy.host.container_manager.credentials._read_git_identity", return_value=(None, None), ), + patch( + "pynchy.host.orchestrator.workspace_config.load_resolved_config", + return_value=fake_resolved, + ), ): env_dir = _write_env_file(is_admin=False, group_folder="code-improver") assert env_dir is not None @@ -308,6 +313,7 @@ def test_non_admin_without_repo_access_gets_no_token(self, tmp_path: Path): }, secrets=MagicMock(gh_token=SecretStr(BROAD_TOKEN)), ) + fake_resolved = MagicMock(repo_access=None) with ( patch("pynchy.host.container_manager.credentials.get_settings", return_value=s), patch("pynchy.host.container_manager.gateway.get_gateway", return_value=None), @@ -315,6 +321,10 @@ def test_non_admin_without_repo_access_gets_no_token(self, tmp_path: Path): "pynchy.host.container_manager.credentials._read_git_identity", return_value=("Test", "test@test.com"), ), + patch( + "pynchy.host.orchestrator.workspace_config.load_resolved_config", + return_value=fake_resolved, + ), ): env_dir = _write_env_file(is_admin=False, group_folder="basic-group") assert env_dir is not None @@ -338,6 +348,7 @@ def test_non_admin_with_repo_access_no_token_configured(self, tmp_path: Path): }, secrets=MagicMock(gh_token=SecretStr(BROAD_TOKEN)), ) + fake_resolved = MagicMock(repo_access=REPO_SLUG) with ( patch("pynchy.host.container_manager.credentials.get_settings", return_value=s), patch("pynchy.host.container_manager.gateway.get_gateway", return_value=None), @@ -345,6 +356,10 @@ def test_non_admin_with_repo_access_no_token_configured(self, tmp_path: Path): "pynchy.host.container_manager.credentials._read_git_identity", return_value=("Test", "test@test.com"), ), + patch( + "pynchy.host.orchestrator.workspace_config.load_resolved_config", + return_value=fake_resolved, + ), ): env_dir = _write_env_file(is_admin=False, group_folder="code-improver") assert env_dir is not None