test(parity): two-tier audit parity test suite (Fixes #366) [stacked on #365] #370
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| # Feature 028 Tier 2: coding-agent skill vs raw MCP tool output parity. | ||
| # | ||
| # Manual-dispatch only. See contract at | ||
| # specs/028-audit-parity-tests/contracts/tier2-workflow.md | ||
| # and governance rationale at spec.md FR-007 / FR-007a / FR-007b. | ||
| # | ||
| # Key security properties (see contract T2-1..T2-16): | ||
| # - T2-1: workflow_dispatch is the ONLY trigger. No schedule; no push. | ||
| # - T2-2: Environment `parity-tier2` is REQUIRED. Configure in GitHub UI | ||
| # with a reviewer list AND the ANTHROPIC_API_KEY secret at the | ||
| # Environment level (NOT repo level). | ||
| # - T2-4: NO other workflow in .github/workflows/ references | ||
| # secrets.ANTHROPIC_API_KEY. Verified by | ||
| # tests/darnit/parity/tier2/test_workflow_config.py. | ||
| # - T2-5: permissions: contents: read only. No write scope granted. | ||
| # - T2-7/T2-8: Preflight actor+SHA logged BEFORE the SDK step consumes | ||
| # the API key. | ||
| # - T2-10: NO api_key workflow input (governance regression guard). | ||
| # - T2-11: artifact upload runs on any exit code (`if: always()`). | ||
| # | ||
| # Kusari Inspector hardening (2026-08-12): | ||
| # - Actions pinned to full commit SHAs (mutable version tags would allow | ||
| # a compromised action owner to silently repoint a tag and execute | ||
| # arbitrary code in the runner while ANTHROPIC_API_KEY is in env). | ||
| # - All `${{ ... }}` context expressions in `run:` blocks moved to | ||
| # step-level `env:` variables and referenced as quoted shell vars. | ||
| # Prevents shell injection via crafted `fixture_glob` inputs. | ||
|
|
||
| name: Parity Tier 2 | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| fixture_glob: | ||
| description: "Glob to filter which fixtures are run (default: all)" | ||
| default: "*" | ||
| required: false | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| tier2: | ||
| runs-on: ubuntu-latest | ||
| environment: parity-tier2 # T2-2: gated Environment with required reviewers | ||
| permissions: | ||
| contents: read # T2-5: no write scope | ||
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 | ||
|
|
||
| - name: Set up Python | ||
| uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 | ||
| with: | ||
| python-version: "3.12" | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@d0cc045d04ccac9d8b7881df0226f9e82c39688e # v6.8.0 | ||
|
|
||
| - name: Sync Tier 2 parity environment | ||
| # PR #370 review fix: `claude-agent-sdk` no longer ships in the | ||
| # `[dev]` extra of `darnit-mcp`; it lives in the dedicated | ||
| # `[parity-tier2]` extra so `pip install darnit-mcp[dev]` on a | ||
| # regular contributor's machine stays lean. Tier 2 CI opts in | ||
| # explicitly. | ||
| run: uv sync --extra dev --extra parity-tier2 | ||
|
|
||
| - name: Preflight audit log (T2-7/T2-8) | ||
| env: | ||
| ACTOR: ${{ github.actor }} | ||
| SHA: ${{ github.sha }} | ||
| FIXTURE_GLOB: ${{ inputs.fixture_glob }} | ||
| run: | | ||
| { | ||
| echo "## Tier 2 Preflight" | ||
| echo "" | ||
| echo "- actor: $ACTOR" | ||
| echo "- sha: $SHA" | ||
| echo "- fixture_glob: $FIXTURE_GLOB" | ||
| echo "- timestamp: $(date -u +%Y-%m-%dT%H:%M:%SZ)" | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
|
|
||
| - name: Run Tier 2 parity check | ||
| env: | ||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||
| FIXTURE_GLOB: ${{ inputs.fixture_glob }} | ||
| run: | | ||
|
There was a problem hiding this comment. Issue: The Run Tier 2 parity check step interpolates Recommended Code Changes: |
||
| # Run as a module (`python -m`) so the `tests.` package imports | ||
| # inside run.py resolve. Running as a script (`python <path>`) | ||
| # would fail because the `tests` package wouldn't be on sys.path. | ||
| # FIXTURE_GLOB is passed via env (not interpolated directly into | ||
| # this shell block) to prevent shell injection. | ||
| uv run python -m tests.darnit.parity.tier2.run \ | ||
| --fixture-glob "$FIXTURE_GLOB" | ||
|
|
||
| - name: Upload parity artifacts (T2-11) | ||
| if: always() | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 | ||
| with: | ||
| name: parity-artifacts | ||
| path: parity-artifacts/ | ||
| retention-days: 30 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| {"feature_directory": "specs/028-audit-parity-tests"} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| # Specification Quality Checklist: Two-Tier Audit Parity Tests | ||
|
|
||
| **Purpose**: Validate specification completeness and quality before proceeding to planning | ||
|
|
||
| **Created**: 2026-08-09 | ||
|
|
||
| **Feature**: [spec.md](../spec.md) | ||
|
|
||
| ## Content Quality | ||
|
|
||
| - [X] No implementation details (languages, frameworks, APIs) | ||
| - [X] Focused on user value and business needs | ||
| - [X] Written for non-technical stakeholders | ||
| - [X] All mandatory sections completed | ||
|
|
||
| ## Requirement Completeness | ||
|
|
||
| - [X] No [NEEDS CLARIFICATION] markers remain | ||
| - [X] Requirements are testable and unambiguous | ||
| - [X] Success criteria are measurable | ||
| - [X] Success criteria are technology-agnostic (no implementation details) | ||
| - [X] All acceptance scenarios are defined | ||
| - [X] Edge cases are identified | ||
| - [X] Scope is clearly bounded | ||
| - [X] Dependencies and assumptions identified | ||
|
|
||
| ## Feature Readiness | ||
|
|
||
| - [X] All functional requirements have clear acceptance criteria | ||
| - [X] User scenarios cover primary flows | ||
| - [X] Feature meets measurable outcomes defined in Success Criteria | ||
| - [X] No implementation details leak into specification | ||
|
|
||
| ## Notes | ||
|
|
||
| - This spec is diagnostic, not remedial: it defines a test surface that WILL detect drift between the three audit consumers. It does NOT define fixes for any specific drift the tests might discover. That intentional scoping keeps the feature small and its purpose clear. | ||
| - Constitution IV echo: the whole reason this feature exists is that a downstream layer (the `/darnit-audit` coding-agent skill) was observed silently reinterpreting the tool's verdicts. Tier 2 makes that behavior visible; a fix for it would be a separate feature. | ||
| - Feature dependencies: 026 (harness) is hard-required for Tier 1. 027 (interactive resolvers) is intentionally out of scope; parity tests do not exercise interactive answer collection. | ||
| - Two areas were considered for [NEEDS CLARIFICATION] but resolved with defaults instead: | ||
| - Tier 2 cadence: chose "nightly or weekly, plan-phase decides." A specific number would over-fit the spec. | ||
| - Claude Agent SDK vs Claude Code CLI subprocess: chose "SDK if available, CLI subprocess as fallback." The plan phase pins the exact choice. | ||
| - The spec commits to closing issue #366 on merge (FR-016 + SC-009). This is the audit trail linking the surfaced problem to the shipped diagnostic. | ||
|
|
||
| ## Clarification Session Log | ||
|
|
||
| Five clarifications recorded during the 2026-08-09 clarify session: | ||
|
|
||
| 1. **Tier 2 invocation mechanism** -> Claude Agent SDK (test-only dep). Follow-up issue #368 opened for OpenAI-SDK and other-provider parity checks; those are separate features. | ||
| 2. **Skill's summary artifact** -> Final assistant message. Diagnostic feature must compare user-facing output; structured-artifact alternatives rejected as intrusive. | ||
| 3. **Tier 2 cadence** -> Manual-only for MVP (`workflow_dispatch`); no schedule. Governance driver: repo is under neutral governance, API key belongs to a specific company. FR-007a + FR-007b + SC-005a lock down the access-control shape. Follow-up issue #369 opened for adding scheduled cadence + governance-appropriate key-sourcing. | ||
| 4. **Tier 1 MCP-tool call shape** -> Direct Python function call (`audit_openssf_baseline(...)`). No MCP server bootstrap; JSON-RPC serialization is a separate concern. | ||
| 5. **Fixture metadata format** -> `parity.toml` at each fixture root, TOML-parsed. Matches Constitution III convention; no code execution; stdlib `tomllib`. | ||
|
|
||
| Two governance-motivated additions surfaced from Q3: | ||
|
|
||
| - FR-007a: Environment-gated dispatch, reviewer-list required, repo-level secret exposure forbidden. | ||
| - FR-007b: Operator-provided API-key inputs forbidden in MVP. | ||
| - SC-005a: Grep-verifiable: no other workflow references `secrets.ANTHROPIC_API_KEY` outside the gated Tier 2 workflow. | ||
|
|
||
| ## /speckit-analyze findings applied | ||
|
|
||
| The 2026-08-10 analyze pass surfaced 8 findings (0 CRITICAL, 1 HIGH, 4 MEDIUM, 3 LOW). Applied remediations: | ||
|
|
||
| - **HC1 (git-init in Tier 1 conftest)**: T012 updated with explicit `prepared_fixture` shape mirroring feature 026's `minimal_llm_repo_tree` pattern. Load-bearing -- without this, the Tier 1 harness invocation would fail before running any control. | ||
| - **MC1 (FR-010 missing-key test)**: T025 gains a `test_missing_api_key_raises_setup_error` subtest with both unit-level (`invoke_skill` raises) and integration-level (`run.py` subprocess exit code 3) assertions. | ||
| - **MC2 (FR-013 green-run summary)**: T013 gains a `capsys.readouterr()` capture + regex-pattern assertion on the summary line, run for EVERY test (green or red). | ||
| - **MC3 (FR-014 no product code changes)**: New task T031a creates `tests/darnit/parity/tier1/test_no_product_changes.py` that runs `git diff --name-only <base>...HEAD` and asserts no file under `packages/darnit/src/` or `packages/darnit-baseline/src/` is modified. Skips on local dev when no base ref is reachable. | ||
| - **MC4 (FR-015 determinism)**: T006 gains a "run compare() twice, assert byte-identical outputs" subtest. | ||
| - **LC1 (allowed-drift wildcard resolution)**: T014's allowed-drift positive cases expanded from just PENDING_LLM->WARN to all three (PENDING_LLM -> WARN | PASS | FAIL). | ||
| - **LC2 (grep portability)**: T024 replaces `subprocess.run(["grep", ...])` with pure-Python file iteration. | ||
| - **LC3 (T024/T031 redundancy)**: No action taken. T024 is the automated test; T031 is the maintainer sanity ritual. Both are cheap; keep both. | ||
|
|
||
| Coverage after remediation: 30/30 requirements have >=1 task; 30/30 have >=1 test task (or manual sign-off for the two doc-shaped ones -- SC-009 via PR body, T033). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| # Contract: `parity.toml` Schema | ||
|
|
||
| **Feature**: 028-audit-parity-tests | **Consumers**: fixture authors adding new corpus entries. | ||
|
|
||
| ## 1. Location | ||
|
|
||
| - **PT-1**: `parity.toml` lives at the root of a fixture directory (`tests/darnit/parity/fixtures/<name>/parity.toml`). Never elsewhere. | ||
| - **PT-2**: `parity.toml` is OPTIONAL. A fixture without one still participates in inter-path parity assertions. | ||
|
|
||
| ## 2. Parsing | ||
|
|
||
| - **PT-3**: Parsed by stdlib `tomllib.load()`. No custom TOML parser; no code execution at load time. | ||
| - **PT-4**: If `parity.toml` exists but is unparseable TOML, the fixture's Tier 1 test FAILS with a "malformed metadata" error. The fixture is not silently skipped. | ||
|
|
||
| ## 3. Schema | ||
|
|
||
| ### 3.1 `[expected]` section | ||
|
|
||
| Required top-level table when `parity.toml` exists. | ||
|
|
||
| ```toml | ||
| [expected] | ||
| category = "mixed" # required; one of "all_pass" | "all_fail" | "mixed" | "pending_llm" | ||
| has_pending_llm = true # optional; auto-derived from counts.pending_llm > 0 if absent | ||
| strict = false # optional; default false | ||
| ``` | ||
|
|
||
| - **PT-5**: `category` MUST be one of the four literal strings. Any other value fails validation. | ||
| - **PT-6**: `has_pending_llm`, when explicitly set, MUST agree with `counts.pending_llm > 0` (if `counts` is present). Disagreement is a validation error. | ||
| - **PT-7**: `strict` controls whether `counts` mismatches (see below) FAIL or WARN. | ||
|
|
||
| ### 3.2 `[expected.counts]` sub-section | ||
|
|
||
| Optional. When present, provides expected control-status distribution. | ||
|
|
||
| ```toml | ||
| [expected.counts] | ||
| pass = 3 | ||
| fail = 2 | ||
| warn = 1 | ||
| error = 0 | ||
| n_a = 0 | ||
| pending_llm = 1 | ||
| ``` | ||
|
|
||
| - **PT-8**: Every key MUST be a non-negative integer. | ||
| - **PT-9**: Unrecognized keys log a warning but do not fail validation (forward-compat). | ||
| - **PT-10**: When `strict = true`, the actual counts from a live audit MUST equal the declared counts, or the fixture's Tier 1 test FAILS. | ||
| - **PT-11**: When `strict = false` (default), a mismatch produces a non-fatal note in the pytest output (informational; useful when a control's status changes due to an upstream `openssf-baseline.toml` update). | ||
|
|
||
| ### 3.3 `[[expected.controls]]` array | ||
|
|
||
| Optional. Per-control expectations for specific controls. | ||
|
|
||
| ```toml | ||
| [[expected.controls]] | ||
| id = "OSPS-GV-01.01" | ||
| status = "PASS" | ||
|
|
||
| [[expected.controls]] | ||
| id = "OSPS-BR-06.01" | ||
| status = "FAIL" | ||
| ``` | ||
|
|
||
| - **PT-12**: `id` MUST match a control the audit produces for this fixture; otherwise validation warns. | ||
| - **PT-13**: `status` MUST be one of the six PassOutcome literals. `PENDING_LLM` is allowed here for the pending_llm category. | ||
| - **PT-14**: The actual status from BOTH the MCP tool and the harness (modulo the PENDING_LLM allowed drift) MUST equal `status`. Mismatch fails when `strict = true`; notes when `strict = false`. | ||
|
|
||
| ## 4. Discovery + iteration | ||
|
|
||
| - **PT-15**: `tier1/fixture_meta.py` provides `load_parity_metadata(fixture_dir: Path) -> ParityMetadata | None`. Returns `None` if `parity.toml` is absent. | ||
| - **PT-16**: `test_corpus_inventory.py` (SC-008) iterates fixtures, calls `load_parity_metadata`, and counts fixtures per `category`. Passes iff at least one fixture is present in each of the four categories. | ||
|
|
||
| ## 5. Example: all_pass_repo | ||
|
|
||
| ```toml | ||
| [expected] | ||
| category = "all_pass" | ||
| has_pending_llm = false | ||
| strict = false | ||
|
|
||
| [expected.counts] | ||
| pass = 8 | ||
| fail = 0 | ||
| warn = 0 | ||
| error = 0 | ||
| n_a = 4 | ||
| pending_llm = 0 | ||
| ``` | ||
|
|
||
| ## 6. Example: pending_llm_repo | ||
|
|
||
| ```toml | ||
| [expected] | ||
| category = "pending_llm" | ||
| has_pending_llm = true | ||
|
|
||
| [expected.counts] | ||
| pass = 4 | ||
| fail = 2 | ||
| warn = 1 | ||
| error = 0 | ||
| n_a = 5 | ||
| pending_llm = 1 | ||
|
|
||
| [[expected.controls]] | ||
| id = "STAGE1-REF-SECURITY-01" | ||
| status = "PENDING_LLM" | ||
| ``` | ||
|
|
||
| ## 7. What `parity.toml` MUST NOT be used for | ||
|
|
||
| - **PT-17**: MUST NOT declare allowed drift beyond the canonical Tier 1 table (see `tier1-parity-invariant.md`). If a fixture legitimately needs a different drift class, that is a spec change, not a fixture change. | ||
| - **PT-18**: MUST NOT influence what controls run. Fixtures use `.baseline.toml` for that; `parity.toml` is test-side metadata only. | ||
| - **PT-19**: MUST NOT contain executable content, template placeholders, or references to environment variables. |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue: The Preflight audit log step interpolates
${{ inputs.fixture_glob }}and${{ github.actor }}directly into the shell script. Move all context values to step-levelenv:variables and reference them as shell variables to prevent injection.Recommended Code Changes: