Skip to content

feat(cli): add suite runner for scenario and trace directories (#85) - #154

Merged
mertsatilmaz merged 2 commits into
OWASP:mainfrom
nidheesh-p:feature/suite-runner
Jul 27, 2026
Merged

mertsatilmaz merged 2 commits into
OWASP:mainfrom
nidheesh-p:feature/suite-runner

Conversation

@nidheesh-p

Copy link
Copy Markdown
Contributor

Closes #85.

Summary

Adds an agent-harness suite subcommand that runs a directory of scenarios against a directory of trace files and produces one aggregate result. Single-scenario run behavior is unchanged.

agent-harness suite scenarios/ --trace-dir traces/ --out-dir results/ --exit-on-fail
  • Discovery: reuses the same files/directories/globs logic as validate.
  • Mapping: each scenario maps to <trace-dir>/<scenario_id>.json by id.
  • Output: per-scenario result JSON into --out-dir plus an aggregate summary.json, and the summary is always printed to stdout. Validated against the new schemas/suite_result.schema.json.
  • Gating: --exit-on-fail exits 1 on any fail/error, composing the same way as run. An empty match or missing --trace-dir exits 1 rather than passing vacuously.

Acceptance criteria (#85)

  • Minimal suite command that keeps single-scenario behavior intact (new suite subcommand; run untouched)
  • Trace-file based suites
  • Per-scenario result JSON and an aggregate summary
  • Exit behavior composes with --exit-on-fail
  • Tests for partial failures and missing trace mappings
  • Documented directory conventions (docs/ci-github-actions.md)

Security / robustness hardening

This went through a security/governance review before implementation:

  • Path traversal: scenario ids are used as path components, so ids are now constrained to [A-Za-z0-9._-] (Python validator + scenario.schema.json), with a path-containment check in the runner as defense-in-depth. A trace lookup can never escape --trace-dir.
  • Duplicate ids, invalid scenario YAML, malformed trace JSON, and missing traces are each recorded as a per-scenario error (with an error_reason) and the suite continues — one broken input never hides the rest.
  • The summary carries per-status counts and provenance (trace path, severity, category) so it is a self-contained audit record; not_run counts are surfaced so a green suite cannot hide a suite that tested nothing.

Testing

  • 368 tests pass (new CLI integration tests, runner unit tests, and a suite_result.schema.json contract test).
  • ruff check and mypy clean.
  • Manual end-to-end run confirmed mixed pass/fail/error aggregation, --exit-on-fail exit codes, out-dir writes, and missing-trace resilience.

AI-assisted contribution disclosure

  • Tool: Claude Code (Opus 4.8).
  • AI-assisted parts: implementation of the suite command, result models, schema, tests, and docs.
  • Review: the author reviewed all output, ran the full test suite, ruff, mypy, and a manual end-to-end smoke test; the design was shaped by an explicit security/governance review pass.

🤖 Generated with Claude Code

Add an `agent-harness suite` subcommand that runs a directory of scenarios
against trace files and emits one aggregate summary plus optional
per-scenario result JSON. Single-scenario `run` is unchanged.

- Map each scenario to `<trace-dir>/<scenario_id>.json` by id.
- Constrain scenario ids to a filename-safe charset and add a path
  containment check so a trace lookup can never escape `--trace-dir`.
- Detect duplicate scenario ids; record per-scenario errors (missing
  trace, malformed trace, invalid scenario, duplicate id) without
  aborting the suite.
- Fail an empty match / missing trace dir rather than passing vacuously.
- Emit per-status counts and provenance (trace path, severity, category)
  in the summary, validated against schemas/suite_result.schema.json.
- `--exit-on-fail` gates on any fail/error, composing with `run`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@nidheesh-p
nidheesh-p requested a review from mertsatilmaz as a code owner June 19, 2026 21:20
# Conflicts:
#	CHANGELOG.md
#	tests/test_cli.py

@mertsatilmaz mertsatilmaz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed in depth — this is excellent work, @nidheesh-p, and thank you for your patience while it sat in my queue.

The design decisions are the ones I'd have asked for: a separate suite subcommand so single-scenario run is untouched, discovery reusing the same files/dirs/globs logic as validate, per-scenario error entries (missing_trace, invalid_scenario, malformed_trace, duplicate id) that record the failure without aborting the rest of the suite, --exit-on-fail composing exactly like run, and a new suite_result.schema.json that correctly carries "version": 1 per the policy from #143. I especially liked that empty matches and a missing --trace-dir exit 1 rather than passing vacuously — that's the failure mode that makes suite runners dangerous in CI.

I pushed three changes to your branch rather than sending it back for a round-trip:

  1. Merged main and resolved conflicts with #153/#157/#158 (CHANGELOG bullets and an interleaved tests/test_cli.py append).
  2. Bumped schemas/scenario.schema.json to "version": 2. Constraining id to [A-Za-z0-9._-] narrows a previously-valid input, which docs/schema-versioning.md classifies as a breaking schema change requiring a version bump. Your ### Changed entry already described the change well — I extended it to record the bump and to note that the pre-1.0 rule means the package major line doesn't move. The constraint itself is right, and mirroring it into both the schema and the Python validator is required by our schema/validator sync tests.
  3. Applied the containment check to the write path. resolve_within guarded the trace read (<trace-dir>/<id>.json) but --out-dir writes used a bare out_dir / f"{id}.json", even though the helper's own docstring promises the suite "must never read or write outside the configured directory". Now both paths go through it. Since it's used across modules I promoted it from _resolve_within to resolve_within.

Verified locally: ruff, mypy, and 378 passing tests; a full 20-scenario suite run against the bundled fixtures (18 pass / 2 not_run / 0 error, summary.json + 20 per-scenario files); the missing-trace path recording 19 missing_trace errors without aborting; --exit-on-fail returning 1 while the flagless run returns 0; and the traversal guard rejecting ../escape.json, ../../etc/passwd, and sub/../../out2.json. CI is green.

Approving and squash-merging. Closes #85 — and with it the last open item on the v0.2.0 milestone.

@mertsatilmaz
mertsatilmaz merged commit 3d094e9 into OWASP:main Jul 27, 2026
3 checks 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.

Add suite-level runner for scenario and trace directories

2 participants