feat(record_cli): serve a different canned response per invocation

[alexandrujircan]

Problem

A record_cli shim answered every invocation with one fixed exit_code / stdout / stderr. So an agent whose next step depends on what the tool just told it could not be evaluated: uip ixp dummy1 and uip ixp dummy2 got the same reply, and anything needing a real answer fell back to a hand-written mock under mock_path_dirs.

What you can write now

sandbox:
  record_cli:
    - tool: uip
      exit_code: 1                    # fallback: anything no rule claims
      stderr: "uip: unknown command\n"
      responses:
        - when: {verb: "ixp dummy1"}
          stdout: "response1\n"
        - when: {verb: "ixp dummy2"}
          stdout: "response2\n"
        - when:                       # any cli_called facet, ANDed
            verb: "ixp projects get"
            positional: ["proj-1"]
            flags: {output: json}
          stdout: '{"id": "proj-1", "name": "Invoices"}'
        - when: {verb: "ixp projects get missing"}
          exit_code: 4
          stderr: "project not found\n"

Rules are tried in declaration order, first match wins, and anything unclaimed gets the entry's own three fields. exit_code defaults to 0 on a rule — the opposite of the entry default of 1 — because a rule exists precisely because the author described that invocation. The log now carries "rule": <index> when a rule answered and omits the key when none did, so "returned the default" and "rule 2 answered, and looks like the default" are no longer the same line.

when is not a second pattern language

The criterion's matcher moved to src/coder_eval/argv_match.py — stdlib-only, plain dicts — and both surfaces lower to one spec dict:

• cli_called calls argv_matches(criterion.match_spec, argv); the checker lost ~160 lines.
• render_recorder embeds that module's source into the shim (read as a package resource), so the pattern that serves a response is the pattern that grades it. A test asserts the embedded copy is the shipped source verbatim; another asserts it is embedded only when the entry declares rules, so a shim with no rules is byte-identical to before.

CE047 (new lint rule) keeps that module's imports stdlib-only: the shim runs where coder_eval and its dependencies are not installed, and one package import there would make every shadowed CLI die with an ImportError the agent reads as "the tool is broken".

FlagMatch moved to the new cycle-free leaf models/cli_match.py, alongside CliMatch and the shared verb/flag validators — models/sandbox.py cannot import from models/criteria.py, which already takes RECORD_CLI_LOG from it.

Two deliberate divergences from the criterion, both pinned by tests in tests/test_cli_match_parity.py:

	cli_called	response rule
ignore_flags default	["output"] — grading must not depend on a flag that changes nothing	[] — dispatch may legitimately answer differently for --output json
tool	a match facet (addresses a log record)	n/a — the shim knows which tool it is

Drive-by fix: a flag inside a verb matched nothing, silently

verb: "ixp projects get --output json" validated and then could never match, because a verb is compared against the non-flag arguments. Silent in the worst direction: cli_called scored 0 against a log holding that exact call. Pre-existing on the criterion; now a validation error on every surface, naming the fix. It reuses the splitter's own is_number rule, so head -1 stays legal.

Review notes

• when is mapping-only — a bare when: "ixp dummy1" is rejected with the {verb: ...} spelling in the message. An earlier draft accepted the string as shorthand; it was dropped so a pattern has one shape.
• FlagMatch keeps its scalar shorthand (flags: {output: json} == {equals: json}). A single-valued predicate has only one facet a scalar could mean, and removing it would be a breaking change to every existing task. Happy to revisit separately if we want strict one-way-only.
• Responses are stateless: a rule answers the same way however many times it matches. A per-call sequence needs a counter file that two concurrent agent commands would race on — that stays a hand-written mock.

Verification

ruff format, ruff check, pyright, and the lint-marked suite are clean on the touched files. 4660 tests pass. Eight failures on my Windows box are pre-existing and unrelated — verified identical at origin/main with these changes stashed: symlink-privilege (4), float.numerator in test_reports_stats_nonfinite (3), and a UTF-8 decode in the CE033 drift test (1).

🤖 Generated with Claude Code

[github-actions]

Claude finished @alexandrujircan's task in 2m 27s —— View job

---

Code Review in Progress

• Read review guidelines (.github/code_review.md)
• Analyze the full diff
• Review architectural changes
• Check argv_match.py module and pattern matching
• Review new lint rule CE047
• Verify test coverage and parity tests
• Check for security and performance implications
• Provide structured feedback

[alexandrujircan]

Thanks — that review found a real defect chain, not a hypothetical one. All two blockers and all five non-blocking items are addressed in dbc5afb.

Blockers

1 + 2 (one chain). FlagMatch.matches_regex now compiles in the model validator, so both surfaces refuse an uncompilable pattern at load:

FlagMatch.matches_regex is not a valid regex with flags=0: unterminated character set at position 1

That makes the checker's pre-flight unreachable, so it is deleted, and its two tests moved from check-time to load-time (including the flags: 99999999 case). Your diagnosis of why it had to move was the decisive part: the criterion could report, the rule surface could not.

For the second half — when the shim's rule evaluation does raise — respond() now returns the error and record() books it as rule_error, so the fault is no longer byte-identical to a clean no-match. cli_called fails the whole log on it, the way it already fails on the .error sentinel:

Recorder could not evaluate its response rules on 1 invocation(s), so the agent saw
fallback output the task did not describe. First: "TypeError('int' object is not iterable)"

I did not make select_rule continue past a faulting rule. Once one rule cannot be evaluated, the responses the agent saw were not the ones the task described, so the honest outcome is failing the log rather than scoring a partially-correct dispatch. Tested by corrupting a rendered shim — the only route left now that such a pattern cannot load.

Non-blocking

1. Dead needs_value — deleted. argv_match.predicate_needs_value now states it is the only implementation and says why a pydantic-side twin is undesirable, rather than claiming a mirror. I did not widen CE037 to public model members: the false-positive surface (properties read only from templates and reports) looked wider than the defect class. Happy to be overruled.
2. Untyped seam — now MatchSpec / FlagPredicate / ResponseRule TypedDicts, required keys indexed directly instead of .get(...) or <default>. Your ignore_flags rename now fails typechecking. Two tests pin that the TypedDict key sets equal the model field sets, which is what makes the single cast honest. record narrowed to dict[str, object] to match parse_log.
3. One-directional parity — added assert set(CliMatch.model_fields) == set(MATCH_FACET_FIELDS). Your env mutation now fails.
4. Shim invariants on one shape only — parametrized over both, and you were right that the spliced shape violated the ASCII one. argv_match.py is ASCII-only now, stated as a rule in its own docstring rather than left to luck.
5. Shadowed rules accepted silently — now a load error, deliberately narrow, since "A matches everything B matches" is not decidable in general. Two sound cases: an exact duplicate, and a verb-only earlier rule whose verb prefixes a later one under the same flag parsing. Your value_flags example is exactly why the parsing clause is there — with different value_flags, a leading --folder F shifts the positionals and the earlier rule does not in fact claim the later one. Five reachable arrangements are pinned as still-accepted.

Nits

All taken: doubled paren fixed; MergeField → Field on responses with the test row dropped and the description corrected (you were right that RecordedCli is never a merge root, so it was inert metadata reading as a knob); CE047 now also reserves SHIM_GLOBALS and derives its target set from invocation_log.EMBEDDED_MODULES, with a test asserting it matches a file that exists; the guide's example uses a non-output flag and now says outright that flags: {output: ...} is the one thing not copy-pastable between the two surfaces.

BREAKING CHANGE footer added to dbc5afb, covering both the flag-in-verb rejection and the matches_regex move to load time.

Verification: ruff format, ruff check, pyright, and the lint suite are clean; 4677 pass. The same 8 failures as before are pre-existing and unrelated (Windows symlink privilege, float.numerator, and the CE033 drift test's own missing encoding=) — identical set at origin/main with this branch stashed.

[Copilot]

🟡 Changes recommended

Two critical shim-safety issues and a moderate response-rule validation issue remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds ordered, per-invocation canned responses to record_cli using shared CLI argument-matching semantics.

Changes:

• Adds response rules, fallback behavior, and rule logging.
• Shares matching and validation with cli_called.
• Adds CE047 lint enforcement, documentation, and tests.

File summaries

File	Review
tests/test_sandbox_record_cli.py	Tests response dispatch and fallback. nit (1 vote): Use “infer” after “leaves which ... to.”
tests/test_custom_lint.py	Tests CE047 behavior.
tests/test_cli_match_parity.py	Tests matching parity.
tests/test_cli_called_criterion.py	Updates matcher and validation tests.
tests/lint/runner.py	Registers CE047.
tests/lint/rules/ce047_embedded_shim_stdlib_only.py	Implements CE047. critical (2 votes): Imported bindings can bypass collision checks and overwrite shim globals; pass bound import names through _check_name.
src/coder_eval/sandbox.py	Reports generated response-rule counts.
src/coder_eval/models/sandbox.py	Adds response rules and reachability validation. moderate (2 votes): The reachability check can reject reachable rules when later flag predicates alter parsing.
src/coder_eval/models/criteria.py	Integrates shared matching definitions.
src/coder_eval/models/cli_match.py	Defines match models and validation.
src/coder_eval/models/__init__.py	Exports the new models.
src/coder_eval/invocation_log.py	Renders response-aware shims. critical (2 votes): SHIM_GLOBALS omits imported template globals, allowing embedded bindings to overwrite names such as sys; include every template-bound name.
src/coder_eval/criteria/cli_called.py	Uses the shared matcher.
src/coder_eval/argv_match.py	Implements shared argument matching.
docs/TASK_DEFINITION_GUIDE.md	Documents response rules. nit (2 votes): Use “infer” rather than “inference” after “meant to.”
CLAUDE.md	Updates architecture and lint guidance.

Review details

Suppressed comments (1)

tests/test_sandbox_record_cli.py:753

• Use the verb “infer” after “leaves which ... to.”

        to inference, and reads enough like a command line to invite flags."""

• Files reviewed: 16/16 changed files
• Comments generated: 4
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.