Skip to content

Recognize managed test wrappers in chained test reports - #762

Merged
wwind123 merged 1 commit into
mainfrom
codex/chained-managed-test-commands
Sep 10, 2026
Merged

Recognize managed test wrappers in chained test reports#762
wwind123 merged 1 commit into
mainfrom
codex/chained-managed-test-commands

Conversation

@wwind123

Copy link
Copy Markdown
Owner

Summary

Recognize managed agent-loop run-tests invocations within parsed shell clauses, not only at the beginning of an entire test report.

Why

An interrupted coder follow-up on #757 reported:

pwd && git status --branch --short && /home/wwind123/.local/bin/agent-loop run-tests --timeout-seconds 120 --memory-dir /home/wwind123/.cache/coding-review-agent-loop/repos/wwind123-coding-review-agent-loop/memory -- python3 -m pytest tests/test_skill_helpers.py tests/test_decomposition.py tests/test_split_materialization.py -q

The guard incorrectly treated the absolute wrapper executable as a test outside the assigned checkout. The same managed invocation was already accepted without the preceding pwd and git status clauses.

Changes

  • Reuse the existing exact managed-wrapper parser for each parsed clause.
  • Validate the inner command and leading assignments recursively, and retain existing path and live-target checks for surrounding clauses.
  • Keep malformed wrapper options ineligible for the special wrapper exemption.
  • Add regressions for &&, ;, and || chains, repeated wrappers, Python-module wrappers, legacy backticked reports, and invalid targets/options.
  • Document that this is reported-command validation, not shell execution or a sandbox.

No installed tool configuration or reviewer approvals are changed.

Validation

  • Focused guard, runtime, and comment-rendering suites: 338 passed.
  • Full rerun, python3 -m pytest tests/ -q --tb=short: 2,715 passed in 138.62 seconds.
  • git diff --check: passed.
  • An initial full-suite run failed the existing 200 ms PTY timeout test, test_runner_pty_timeout_is_opt_in_and_retains_combined_log. Its isolated rerun passed; no PTY/test-timeout code was changed.

-- OpenAI Codex

@wwind123

Copy link
Copy Markdown
Owner Author

Review verdict: Approved

The clause-level managed-wrapper recognition is correctly scoped: it preserves the wrapper and memory-output exemptions while recursively validating leading assignments and inner commands, continues validating surrounding shell clauses, and falls back to ordinary validation for malformed wrapper syntax. The focused regression coverage exercises both supported wrapper forms, multiple chain operators, structured and legacy reports, outside paths, live targets, and malformed options. GitHub PR checks are passing.

-- OpenAI Codex: gpt-5.6-sol (medium)

@wwind123

Copy link
Copy Markdown
Owner Author

Review verdict: Approved

Verified the local diff at head 0ed9023 in the assigned checkout. Factoring the wrapper contract into _validate_managed_command and applying it per clause in _validate_command_contents is the right shape: the exemption is still limited to the exact wrapper executable and its --memory-dir output, leading shell assignments are validated as command syntax, and the inner argv recurses through the normal path and live-target passes. Recursion terminates because the inner argv is strictly shorter than the recognized wrapper clause, and the per-clause command_by_contract/mode flags are fixed at split time, so dropping recognized clauses from the ordinary lists cannot reclassify neighbouring clauses. Keeping the path pass and URL pass as two separate loops preserves the previous error precedence. Test coverage is good: positive chains across &&, ;, ||, repeated wrappers and both wrapper spellings under structured and response origins, plus negative cases proving a chained wrapper cannot hide cd /outside, an outside inner target, --rootdir=/outside, an assignment or bare live URL, a nested outside wrapper, or an echoed wrapper string, and that malformed options still lose the exemption. Docs in docs/local_agent_loop.md describe the per-clause behaviour and its limits accurately; no user-facing subcommand or flag changed, so README needs no update. GitHub PR checks are reported as passing; I ran no tests myself per the read-only review constraints.

-- Anthropic Claude: claude-opus-5 (medium)

@wwind123

Copy link
Copy Markdown
Owner Author

PR review round 1 reconciliation: settled reviewers: Claude, Codex. Finalization continues after reconciliation.

@wwind123

Copy link
Copy Markdown
Owner Author

Reviewers approved PR #762; watching GitHub checks in the foreground. No coder or reviewer agents will run while checks remain pending.

-- coding-review-agent-loop

@wwind123
wwind123 merged commit 0d130e0 into main Sep 10, 2026
1 check passed
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.

1 participant