Add bounded MAF fan-out and isolated worker execution - #3
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71f9780388
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name="cas_task_worker", | ||
| description="Bounded read-only specialist fan-out with typed terminal fan-in", | ||
| ) | ||
| return builder.add_fan_out_edges(dispatcher, specialists).add_fan_in_edges(specialists, aggregator).build() |
There was a problem hiding this comment.
When this helper is used for the actual MAF workflow, add_fan_out_edges broadcasts the dispatcher message to every specialist, so the required four specialists can run at once; max_iterations does not cap concurrency. That bypasses the three-worker cap enforced by the process adapter and can exceed the intended rate/cost bound for the production MAF path, so add a limiter/gate or route this workflow through the bounded runner.
Useful? React with 👍 / 👎.
| self.contract.validate() | ||
| if self._manifest is None: | ||
| raise RuntimeError("Sandbox has not been created") | ||
| normalized = relative_path.replace("\\", "/").lstrip("./") |
There was a problem hiding this comment.
For mutation requests that start with ../, /, or a dotfile name, lstrip("./") removes the safety markers before the absolute/traversal/blocked checks run, so inputs like ../src/a.py, /src/a.py, and .env become src/a.py or env and can be accepted whenever the allowlist matches. This weakens the sandbox boundary and the secret-path block; validate the raw normalized path before stripping only an exact ./ prefix.
Useful? React with 👍 / 👎.
| pure = PurePosixPath(normalized) | ||
| if not normalized or pure.is_absolute() or ".." in pure.parts or any(part.lower() in _BLOCKED_PARTS for part in pure.parts): | ||
| raise ValueError(f"Write path is blocked: {relative_path}") | ||
| if not any(pure.match(pattern) for pattern in self.contract.path_allowlist): |
There was a problem hiding this comment.
Using PurePosixPath.match here does not enforce that the path starts at the allowed root: with an allowlist such as src/**, a request for foo/src/change.txt is authorized, while common nested owned files like src/pkg/change.txt are not. That breaks the single-owner path contract because workers can write outside their assigned subtree (and be blocked inside it); use anchored relative glob or explicit prefix checks against the worktree-relative path.
Useful? React with 👍 / 👎.
1 participant
Summary\nAdds the MAF task-worker boundary with bounded specialist fan-out, strict fan-in, isolated Git worktrees, and deterministic approval policy.\n\n## Changes\n- Add four read-only specialist roles with peak concurrency three\n- Require complete typed terminal fan-in\n- Add single-owner mutation contracts and Git worktree isolation\n- Deny blocked paths and require approval for external actions\n- Expose a deterministic process adapter for the orchestrator\n\n## Why\nParallel analysis must remain bounded while repository mutation stays isolated, single-owner, and policy-controlled.\n\n## Validation\n- 114 tests passed on the clean origin/main-based branch\n- Real Git worktree isolation test passed\n- Process adapter measured peak concurrency three\n\n## Risks\n- The process adapter is deterministic infrastructure; production model selection remains separately configured\n- Worktree cleanup must remain part of operational lifecycle handling\n\n## Related\nCAS Loop Engineering milestone v1.\n\n## Reviewer notes\nStart with maf_starter/loop_workers.py and maf_starter/loop_sandbox.py.