fix(workerpals): enforce honest scope and recover shell wrappers

Author: PiyushDatta

apps/workerpals/src/backends/openai_codex/openai_codex_executor.py

@@ -96,6 +96,8 @@
 _VALID_AUTH_MODES = {"auto", "api_key", "chatgpt"}
 _VALID_REASONING_EFFORTS = {"low", "medium", "high", "xhigh"}
 _MAX_WRAPPER_RECOVERY_ATTEMPTS = 2
+_MAX_WRAPPER_BOOTSTRAP_OUTPUT_CHARS = 1_200
+_MAX_WRAPPER_BOOTSTRAP_TOTAL_CHARS = 5_000
 
 
 def _model_supports_xhigh_reasoning(model: str) -> bool:
@@ -1108,6 +1110,177 @@ def _build_wrapper_recovery_guidance(rejected_commands: List[str], *, hard: bool
     return "\n".join(guidance_lines)
 
 
+def _truncate_wrapper_bootstrap_output(text: str) -> str:
+    value = str(text or "").replace("\r\n", "\n").strip()
+    if len(value) <= _MAX_WRAPPER_BOOTSTRAP_OUTPUT_CHARS:
+        return value
+    return f"{value[:_MAX_WRAPPER_BOOTSTRAP_OUTPUT_CHARS].rstrip()}\n...(truncated)"
+
+
+def _resolve_repo_scoped_path(repo: str, raw_path: str) -> Optional[Path]:
+    candidate = str(raw_path or "").strip()
+    if not candidate:
+        return None
+    repo_root = Path(repo).resolve()
+    resolved = (repo_root / candidate).resolve()
+    try:
+        common = os.path.commonpath([str(repo_root), str(resolved)])
+    except ValueError:
+        return None
+    if common != str(repo_root):
+        return None
+    return resolved
+
+
+def _run_wrapper_bootstrap_command(repo: str, command: str) -> str:
+    normalized = _normalize_command_text(command)
+    if not normalized:
+        return ""
+    try:
+        args = shlex.split(normalized, posix=True)
+    except ValueError:
+        return ""
+    if not args:
+        return ""
+    program = str(args[0] or "").strip().lower()
+    if program == "pwd" and len(args) == 1:
+        return repo
+    if program == "ls":
+        target = Path(repo).resolve()
+        if len(args) == 2 and not str(args[1]).startswith("-"):
+            resolved = _resolve_repo_scoped_path(repo, str(args[1]))
+            if not resolved:
+                return ""
+            target = resolved
+        elif len(args) > 1:
+            return ""
+        if not target.exists():
+            return f"{target.name or str(target)} (missing)"
+        if target.is_file():
+            return target.name
+        entries = sorted(child.name for child in target.iterdir())
+        return "\n".join(entries[:120])
+    if program == "git" and len(args) >= 2:
+        safe_git_args: Optional[List[str]] = None
+        if args[1:] == ["branch", "--show-current"]:
+            safe_git_args = ["git", "--no-pager", "branch", "--show-current"]
+        elif args[1:] == ["status", "--porcelain"]:
+            safe_git_args = ["git", "--no-pager", "status", "--porcelain"]
+        elif len(args) >= 3 and args[1] == "diff":
+            diff_args = list(args[2:])
+            sanitized_paths: List[str] = []
+            if diff_args == ["--name-only"]:
+                safe_git_args = [
+                    "git",
+                    "--no-pager",
+                    "diff",
+                    "--no-ext-diff",
+                    "--no-textconv",
+                    "--name-only",
+                ]
+            elif len(diff_args) >= 2 and diff_args[0] == "--name-only" and diff_args[1] == "--":
+                for raw_path in diff_args[2:]:
+                    resolved = _resolve_repo_scoped_path(repo, str(raw_path))
+                    if not resolved:
+                        return ""
+                    sanitized_paths.append(os.path.relpath(str(resolved), repo))
+                safe_git_args = [
+                    "git",
+                    "--no-pager",
+                    "diff",
+                    "--no-ext-diff",
+                    "--no-textconv",
+                    "--name-only",
+                    "--",
+                    *sanitized_paths,
+                ]
+            elif diff_args and diff_args[0] == "--":
+                for raw_path in diff_args[1:]:
+                    resolved = _resolve_repo_scoped_path(repo, str(raw_path))
+                    if not resolved:
+                        return ""
+                    sanitized_paths.append(os.path.relpath(str(resolved), repo))
+                safe_git_args = [
+                    "git",
+                    "--no-pager",
+                    "diff",
+                    "--no-ext-diff",
+                    "--no-textconv",
+                    "--",
+                    *sanitized_paths,
+                ]
+        if not safe_git_args:
+            return ""
+        proc = subprocess.run(
+            safe_git_args,
+            cwd=repo,
+            capture_output=True,
+            text=True,
+            timeout=15,
+            check=False,
+        )
+        output = proc.stdout.strip()
+        if proc.returncode != 0:
+            detail = proc.stderr.strip() or output
+            return f"(command failed: {detail})" if detail else "(command failed)"
+        return output
+    if program == "cat" and len(args) == 2:
+        resolved = _resolve_repo_scoped_path(repo, str(args[1]))
+        if not resolved or not resolved.is_file():
+            return ""
+        return resolved.read_text(encoding="utf-8", errors="replace")
+    if program == "sed" and len(args) == 4 and args[1] == "-n":
+        match = re.fullmatch(r"(\d+),(\d+)p", str(args[2] or "").strip())
+        if not match:
+            return ""
+        start = max(1, int(match.group(1)))
+        end = max(start, int(match.group(2)))
+        resolved = _resolve_repo_scoped_path(repo, str(args[3]))
+        if not resolved or not resolved.is_file():
+            return ""
+        lines = resolved.read_text(encoding="utf-8", errors="replace").splitlines()
+        return "\n".join(lines[start - 1 : end])
+    return ""
+
+
+def _build_wrapper_bootstrap_context(repo: str, rejected_commands: List[str]) -> str:
+    blocks: List[str] = []
+    total_chars = 0
+    seen: set[str] = set()
+    for rejected in rejected_commands:
+        direct = _unwrap_shell_wrapper_command(rejected)
+        key = direct.lower()
+        if not direct or key in seen:
+            continue
+        seen.add(key)
+        output = _run_wrapper_bootstrap_command(repo, direct)
+        if not output:
+            continue
+        truncated = _truncate_wrapper_bootstrap_output(output)
+        block = (
+            f"- Direct command: `{direct}`\n"
+            f"  Rejected wrapper: `{rejected}`\n"
+            "  Output:\n"
+            "  ```text\n"
+            f"{truncated}\n"
+            "  ```"
+        )
+        if total_chars + len(block) > _MAX_WRAPPER_BOOTSTRAP_TOTAL_CHARS and blocks:
+            break
+        blocks.append(block)
+        total_chars += len(block)
+    if not blocks:
+        return ""
+    return "\n".join(
+        [
+            "Direct command context bootstrap:",
+            "The backend already ran safe read-only direct replacements for some rejected wrapper commands.",
+            "Use these outputs as current repo context and do not rerun the wrapped variants.",
+            *blocks[:6],
+        ]
+    )
+
+
 def _merge_usage_records(first: Any, second: Any) -> Dict[str, Any]:
     first_record = first if isinstance(first, dict) else {}
     second_record = second if isinstance(second, dict) else {}
@@ -1547,18 +1720,28 @@ def _drain_stderr() -> None:
                     hard=hard_recovery,
                 )
                 if recovery_guidance:
+                    bootstrap_context = (
+                        _build_wrapper_bootstrap_context(repo, rejected_shell_wrappers)
+                        if hard_recovery
+                        else ""
+                    )
                     log.warning(
                         "Codex hit a shell-wrapper rejection loop; retrying once with "
                         + (
                             "strict no-wrapper recovery guidance."
+                            + (" Added direct-command context bootstrap." if bootstrap_context else "")
                             if hard_recovery
                             else "direct-command recovery guidance."
                         )
                     )
                     retry_result = _run_codex_task(
                         repo,
                         instruction,
-                        [*effective_supplemental_guidance, recovery_guidance],
+                        [
+                            *effective_supplemental_guidance,
+                            *( [bootstrap_context] if bootstrap_context else [] ),
+                            recovery_guidance,
+                        ],
                         wrapper_recovery_attempt=wrapper_recovery_attempt + 1,
                         baseline_changes=baseline_snapshot,
                     )

apps/workerpals/src/backends/openai_codex/test_openai_codex_runtime_config.py

@@ -24,6 +24,7 @@
 from openai_codex_executor import (
     OpenAICodexRuntimeConfig,
     _augment_supplemental_guidance,
+    _build_wrapper_bootstrap_context,
     _build_wrapper_recovery_guidance,
     _run_codex_task,
     _resolve_reasoning_effort,
@@ -295,6 +296,56 @@ def test_wrapper_hard_recovery_guidance_requires_direct_replacements_first(self)
         self.assertIn("first command invocation on this retry must be one of the direct replacements", lowered)
         self.assertIn("`/bin/bash -lc 'git status --porcelain'` -> `git status --porcelain`", guidance)
 
+    def test_build_wrapper_bootstrap_context_runs_safe_direct_replacements(self) -> None:
+        with tempfile.TemporaryDirectory(prefix="pushpals-codex-wrapper-bootstrap-") as temp_dir:
+            repo = Path(temp_dir) / "repo"
+            repo.mkdir(parents=True, exist_ok=True)
+            (repo / "README.md").write_text("# wrapper bootstrap test\n", encoding="utf-8")
+            subprocess.run(["git", "init"], cwd=repo, check=True, capture_output=True, text=True)
+            subprocess.run(
+                ["git", "config", "user.name", "PushPals Test"],
+                cwd=repo,
+                check=True,
+                capture_output=True,
+                text=True,
+            )
+            subprocess.run(
+                ["git", "config", "user.email", "pushpals-tests@example.com"],
+                cwd=repo,
+                check=True,
+                capture_output=True,
+                text=True,
+            )
+            subprocess.run(["git", "add", "README.md"], cwd=repo, check=True, capture_output=True, text=True)
+            subprocess.run(
+                ["git", "commit", "-m", "chore: seed wrapper bootstrap repo"],
+                cwd=repo,
+                check=True,
+                capture_output=True,
+                text=True,
+            )
+
+            guidance = _build_wrapper_bootstrap_context(
+                str(repo),
+                [
+                    "/bin/bash -lc pwd",
+                    "/bin/bash -c pwd",
+                    "/bin/bash -lc ls",
+                    "/bin/bash -lc 'git branch --show-current'",
+                    "/bin/bash -lc 'cat README.md'",
+                    "/bin/bash -lc 'git diff --output=leak.txt'",
+                ],
+            )
+
+        self.assertIn("Direct command context bootstrap:", guidance)
+        self.assertIn("Direct command: `pwd`", guidance)
+        self.assertIn("Direct command: `ls`", guidance)
+        self.assertIn("Direct command: `git branch --show-current`", guidance)
+        self.assertIn("Direct command: `cat README.md`", guidance)
+        self.assertIn("README.md", guidance)
+        self.assertIn("# wrapper bootstrap test", guidance)
+        self.assertNotIn("git diff --output=leak.txt", guidance)
+
     def test_run_codex_task_escalates_wrapper_recovery_and_recovers(self) -> None:
         with tempfile.TemporaryDirectory(prefix="pushpals-codex-wrapper-recovery-") as temp_dir:
             repo = Path(temp_dir) / "repo"
@@ -341,13 +392,16 @@ def test_run_codex_task_escalates_wrapper_recovery_and_recovers(self) -> None:
                         "",
                         "prompt = sys.stdin.read()",
                         "hard_marker = 'Your first command invocation on this retry must be one of the direct replacements listed below'",
-                        "if hard_marker in prompt:",
+                        "bootstrap_marker = 'Direct command context bootstrap:'",
+                        "pwd_marker = 'Direct command: `pwd`'",
+                        "branch_marker = 'Direct command: `git branch --show-current`'",
+                        "if hard_marker in prompt and bootstrap_marker in prompt and pwd_marker in prompt and branch_marker in prompt:",
                         "    if last_message_path:",
                         "        Path(last_message_path).write_text(",
-                        "            'Recovered by switching to direct commands after strict wrapper recovery.',",
+                        "            'Recovered by using backend-supplied direct command bootstrap after strict wrapper recovery.',",
                         "            encoding='utf-8',",
                         "        )",
-                        "    print('item.completed | Used direct commands after strict recovery guidance.', flush=True)",
+                        "    print('item.completed | Used backend bootstrap context after strict recovery guidance.', flush=True)",
                         "    sys.exit(0)",
                         "",
                         "for line in (",
@@ -380,6 +434,7 @@ def test_run_codex_task_escalates_wrapper_recovery_and_recovers(self) -> None:
         self.assertTrue(result.get("ok"), result)
         self.assertIn("Recovered after Codex attempts hit command-router shell-wrapper rejections.", str(result.get("stdout") or ""))
         self.assertIn("strict wrapper recovery", str(result.get("stdout") or "").lower())
+        self.assertIn("backend-supplied direct command bootstrap", str(result.get("stdout") or ""))
 
     def test_usage_falls_back_to_estimate_when_trace_has_no_usage(self) -> None:
         usage = _usage_from_trace_or_estimate({}, "abc" * 30, "done", model="gpt-5.4")

apps/workerpals/src/execute_job.ts

@@ -333,6 +333,13 @@ export function shouldEnqueueNoChangeReviewCompletion(
   return extractReviewFixContext(params) == null;
 }
 
+function reviewAgentAllowsMultiRootScope(value: unknown): boolean {
+  const normalized = String(value ?? "")
+    .trim()
+    .toLowerCase();
+  return normalized === "review_fix" || normalized === "merge_conflict";
+}
+
 export function deriveQualityGatePolicy(
   params: Record<string, unknown> | null | undefined,
   runtimeConfig: WorkerpalsRuntimeConfig = DEFAULT_CONFIG,
@@ -3217,6 +3224,7 @@ function validateTaskExecutePlanning(
   options?: {
     origin?: "autonomy" | "user";
     autonomyComponentArea?: unknown;
+    reviewAgentResolutionType?: unknown;
   },
 ): { ok: true } | { ok: false; message: string } {
   if (!value || typeof value !== "object" || Array.isArray(value)) {
@@ -3300,21 +3308,26 @@ function validateTaskExecutePlanning(
     const normalizedWriteGlobs = isStringArray(scope.writeGlobs)
       ? toStringArray(scope.writeGlobs)
       : [];
+    const allowMultiRootAutonomyScope =
+      origin === "autonomy" &&
+      reviewAgentAllowsMultiRootScope(options?.reviewAgentResolutionType);
     if (origin === "autonomy") {
       const declaredComponentArea = asAutonomyComponentArea(options?.autonomyComponentArea);
-      const inferredComponentArea = deriveAutonomyComponentArea(
-        normalizedTargetPaths,
-        normalizedWriteGlobs,
-      );
-      const componentArea = declaredComponentArea ?? inferredComponentArea;
-      if (!componentArea) {
+      const inferredComponentArea = allowMultiRootAutonomyScope
+        ? null
+        : deriveAutonomyComponentArea(normalizedTargetPaths, normalizedWriteGlobs);
+      const componentArea = allowMultiRootAutonomyScope
+        ? declaredComponentArea
+        : declaredComponentArea ?? inferredComponentArea;
+      if (!allowMultiRootAutonomyScope && !componentArea) {
         return {
           ok: false,
           message:
             "task.execute planning.targetPaths must resolve to a repo-relative componentArea",
         };
       }
       if (
+        !allowMultiRootAutonomyScope &&
         declaredComponentArea &&
         inferredComponentArea &&
         declaredComponentArea !== inferredComponentArea
@@ -3328,7 +3341,7 @@ function validateTaskExecutePlanning(
         componentArea,
         normalizedTargetPaths,
         normalizedWriteGlobs,
-        { requireWriteGlobs: false },
+        { requireWriteGlobs: false, allowMultipleComponentRoots: allowMultiRootAutonomyScope },
       );
       if (!validatedScope.ok) {
         return {
@@ -3686,9 +3699,14 @@ export async function executeJob(
     params.autonomy && typeof params.autonomy === "object" && !Array.isArray(params.autonomy)
       ? (params.autonomy as Record<string, unknown>)
       : null;
+  const reviewAgent =
+    params.reviewAgent && typeof params.reviewAgent === "object" && !Array.isArray(params.reviewAgent)
+      ? (params.reviewAgent as Record<string, unknown>)
+      : null;
   const planningValidation = validateTaskExecutePlanning(params.planning, {
     origin,
     autonomyComponentArea: autonomyScope?.componentArea ?? autonomyScope?.component_area,
+    reviewAgentResolutionType: reviewAgent?.resolutionType,
   });
   if (!planningValidation.ok) {
     return {