test(parity): add OpenAI Tier 2 backend + SkillInvocationBackend Protocol (Fixes #368) [stacked on #370] #371
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,110 @@ | ||
| # Feature 029 Tier 2 (OpenAI): coding-agent skill vs raw MCP tool output | ||
| # parity for OpenAI-based invocations. | ||
| # | ||
| # Manual-dispatch only. Environment-gated so an authorized reviewer must | ||
| # approve each run BEFORE OPENAI_API_KEY is exposed. See contract at | ||
| # specs/029-openai-parity-adapter/contracts/openai-workflow.md | ||
| # and governance rationale at spec.md FR-005 / FR-007 / FR-007a-equivalent. | ||
| # | ||
| # Key properties (see contract OW-1..OW-16): | ||
| # - OW-1: workflow_dispatch is the ONLY trigger. No schedule; no push. | ||
| # - OW-2: two inputs -- fixture_glob AND model (pinned version-suffixed default). | ||
| # - OW-3: model default MUST be a version-suffixed string (SC-010). | ||
| # - OW-4: environment `parity-tier2-openai` (distinct from Claude's env). | ||
| # - OW-6: permissions: contents: read only. | ||
| # - OW-7/OW-8/OW-9: OPENAI_API_KEY only in this workflow; the | ||
| # Anthropic-scoped key MUST NOT appear here. | ||
| # - OW-10: preflight actor+SHA+model logged BEFORE the SDK step. | ||
| # - OW-13: no api_key workflow input (governance regression guard). | ||
| # - OW-14: 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 OPENAI_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 or model inputs. | ||
|
|
||
| name: Parity Tier 2 (OpenAI) | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| fixture_glob: | ||
| description: "Glob to filter which fixtures are run (default: all)" | ||
| default: "*" | ||
| required: false | ||
| model: | ||
| description: "Pinned OpenAI model (versioned suffix required; e.g. gpt-4o-2024-08-06)" | ||
| default: "gpt-4o-2024-08-06" | ||
| required: false | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| tier2: | ||
| runs-on: ubuntu-latest | ||
| environment: parity-tier2-openai # OW-4: gated Environment with required reviewers | ||
| permissions: | ||
| contents: read # OW-6: 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 dev environment | ||
| run: uv sync --extra dev | ||
|
|
||
| - name: Preflight audit log (OW-10) | ||
| env: | ||
| ACTOR: ${{ github.actor }} | ||
| SHA: ${{ github.sha }} | ||
| FIXTURE_GLOB: ${{ inputs.fixture_glob }} | ||
| MODEL: ${{ inputs.model }} | ||
| run: | | ||
| { | ||
| echo "## Tier 2 (OpenAI) Preflight" | ||
| echo "" | ||
| echo "- actor: $ACTOR" | ||
| echo "- sha: $SHA" | ||
| echo "- fixture_glob: $FIXTURE_GLOB" | ||
| echo "- model: $MODEL" | ||
| echo "- timestamp: $(date -u +%Y-%m-%dT%H:%M:%SZ)" | ||
| } >> "$GITHUB_STEP_SUMMARY" | ||
|
|
||
| - name: Run Tier 2 parity check (OpenAI) | ||
| env: | ||
| # OW-7: OPENAI_API_KEY appears in this workflow only. | ||
| # OW-9: this workflow deliberately does NOT expose any other | ||
| # provider's key -- the Claude/Anthropic-scoped key is only | ||
| # reachable from the sibling workflow (parity-tier2.yml). | ||
| OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} | ||
| FIXTURE_GLOB: ${{ inputs.fixture_glob }} | ||
| MODEL: ${{ inputs.model }} | ||
| run: | | ||
|
There was a problem hiding this comment. Issue: The run: block at this step has OPENAI_API_KEY in scope and directly interpolates inputs.model and inputs.fixture_glob. Move inputs to env: variables to prevent injection-based secret exfiltration. Recommended Code Changes: |
||
| # Run as a module (`python -m`) so the `tests.` package imports | ||
| # inside run.py resolve. FIXTURE_GLOB and MODEL are passed via env | ||
| # (not interpolated directly into this shell block) to prevent | ||
| # shell injection. | ||
| uv run python -m tests.darnit.parity.tier2.run \ | ||
| --backend openai \ | ||
| --model "$MODEL" \ | ||
| --fixture-glob "$FIXTURE_GLOB" | ||
|
|
||
| - name: Upload parity artifacts (OW-14) | ||
| if: always() | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 | ||
| with: | ||
| name: parity-artifacts-openai | ||
| 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| {"feature_directory": "specs/029-openai-parity-adapter"} |
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,75 @@ | ||
| # Specification Quality Checklist: OpenAI Tier 2 Parity Adapter | ||
|
|
||
| **Purpose**: Validate specification completeness and quality before proceeding to planning | ||
|
|
||
| **Created**: 2026-08-10 | ||
|
|
||
| **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 feature closes issue #368 (opened during feature 028's clarify pass as the follow-up for provider-agnostic Tier 2 checks). | ||
| - Two spec-level decisions were made explicitly in the spec rather than deferred as [NEEDS CLARIFICATION]: | ||
| - **Separate workflow per provider** (FR-005): rather than one aggregate workflow with a `provider` input. This preserves per-provider governance -- the OpenAI Environment has its own reviewer list distinct from the Claude Environment. | ||
| - **Shared skill prompt snapshot** (FR-009): feature 028's `skill_prompt_snapshot.md` is used verbatim by both backends. If provider-specific transformations are required (e.g., differing tool-call syntax), those live in the adapter, not in a forked snapshot. | ||
| - Corners intentionally deferred to /speckit-clarify: | ||
| - Which specific OpenAI API surface (Assistants API vs Chat Completions with tools). Both work; the choice affects turn-loop implementation but not the spec's contract. | ||
| - Whether the `NoopBackend` used to prove SC-005 / SC-007 lives in the tests package or is a documented "how to write a backend" reference. Plan-phase decision. | ||
| - The exact CI cadence question (US3 aggregate reporting) -- Priority 3, out of scope for MVP, no clarify question needed. | ||
| - Constitution IV echo: the OpenAI adapter, like the Claude adapter, MUST NOT modify the `/darnit-audit` skill it diagnoses. Any prompt-shape transformation is adapter-internal. | ||
| - Feature dependencies: feature 028 (parity test suite) is hard-required. Its `SkillReport` parser, `Tier2DiffReport` differ, `write_fixture_artifacts` writer, and `run.py` runner CLI are all consumed by this feature -- extended, but not forked. | ||
|
|
||
| ## Clarification Session Log | ||
|
|
||
| Five clarifications recorded during the 2026-08-10 clarify session: | ||
|
|
||
| 1. **OpenAI API surface** -> Chat Completions with `tools=[...]` and hand-rolled tool-call loop. Stateless per-invocation; symmetric with feature 028's Claude adapter. (FR-001) | ||
| 2. **Backend registration mechanism** -> Simple factory dict in a shared module; no entry-point discovery. TEST-ONLY seam; distinct from feature 027's product-facing `QuestionResolver`. (FR-004) | ||
| 3. **Turn cap exhausted** -> New distinct outcome `turn_cap_exhausted` with exit code `5`. Diagnostically separate from `unparseable` and `per_control_disagree`. (FR-010; SC-011 added) | ||
| 4. **Model default** -> Pin a version-suffixed string in the workflow YAML (e.g., `gpt-4o-2024-08-06`). Reproducibility is load-bearing for a diagnostic; moving aliases forbidden. (SC-010 added) | ||
| 5. **NoopBackend location** -> Test-only fixture at `tests/darnit/parity/tier2/backends/noop.py`; Protocol shape documented in `contracts/skill-invocation-backend-protocol.md` for real backend authors. | ||
|
|
||
| Two new SCs surfaced from these decisions: SC-010 (pinned model check) and SC-011 (turn-cap adversarial test). Coverage after clarify: 17 FR + 11 SC = 28 requirements, all with concrete acceptance criteria. | ||
|
|
||
| ## /speckit-analyze findings applied | ||
|
|
||
| The 2026-08-11 analyze pass surfaced 8 findings (0 CRITICAL, 0 HIGH, 4 MEDIUM, 4 LOW). Applied remediations: | ||
|
|
||
| - **MC1 (FR-013 fixture-diff)**: New task T024a manually verifies no fixture files were modified in this PR. Documented as a soft-constraint pre-PR check rather than a test. | ||
| - **MC2 (FR-014 parser reuse test)**: T016 gains a subtest `test_openai_style_markdown_is_parseable_by_shared_parser` that feeds an OpenAI-shaped Markdown response through feature 028's `SkillReport.parse()` and asserts parseable. | ||
| - **MC3 (shim export inventory)**: New task T009a creates `test_shim_exports.py` that imports every public name from feature 028's original module surface via the shim path. | ||
| - **MC4 (rebase watch list)**: New "Rebase conflict watch list" section in tasks.md enumerates the 6 files most likely to conflict on rebase from feature 028's PR review. | ||
| - **LC1 (T007/T008 ordering)**: Deps chart updated to explicitly state T008 runs before T007. | ||
| - **LC3 (Environment UI callout)**: New "Before-merge maintainer actions" section (M1-M4) documents the manual GitHub UI configuration that no code task performs. | ||
| - **LC4 (T009 canary list)**: T009 gains a pointer to the feature-028 test files most likely to surface a shim regression. | ||
|
|
||
| Not applied: | ||
|
|
||
| - **LC2 (T018 split)**: Would renumber tasks; declined as churn without material benefit. | ||
|
|
||
| Task count after remediation: 32 tasks (T001-T024a, T029 with T009a, T024a intercalated). Coverage after remediation: 27/28 requirements have a concrete task or automated check; 1/28 (SC-009 30-min corpus wall clock) remains manual verification post-merge, as designed. |
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,66 @@ | ||
| # Contract: `parity-tier2-openai.yml` Workflow | ||
|
|
||
| **Feature**: 029-openai-parity-adapter | **Consumers**: maintainers configuring the `parity-tier2-openai` Environment; reviewers approving OpenAI dispatches; auditors verifying access-control compliance. | ||
|
|
||
| Mirrors feature 028's `parity-tier2.yml` contract (`tier2-workflow.md`) for the OpenAI backend. Governance-critical properties are enforced identically. | ||
|
|
||
| ## 1. Trigger | ||
|
|
||
| - **OW-1**: Triggered EXCLUSIVELY by `workflow_dispatch`. No `push`, no `pull_request`, no `schedule`. | ||
| - **OW-2**: Inputs: `fixture_glob` (default `"*"`) AND `model` (default `gpt-4o-2024-08-06` -- SC-010 requires a version-suffixed default). | ||
| - **OW-3**: The `model` input's default MUST be a version-suffixed string. A moving alias (e.g., `gpt-4o` alone) fails the `test_openai_workflow_pins_versioned_model` check (SC-010). | ||
|
|
||
| ## 2. Environment | ||
|
|
||
| - **OW-4**: Job MUST declare `environment: parity-tier2-openai`. Distinct from feature 028's `parity-tier2`. | ||
| - **OW-5**: GitHub UI (NOT this YAML) MUST configure the `parity-tier2-openai` Environment with: | ||
| - A required-reviewer list of authorized maintainers. | ||
| - `OPENAI_API_KEY` stored at the ENVIRONMENT level. | ||
| - No other secrets in this Environment (blast-radius minimization). | ||
|
|
||
| ## 3. Permissions | ||
|
|
||
| - **OW-6**: `permissions: contents: read` at the job level. No `write` scope granted to any resource. | ||
|
|
||
| ## 4. Key exclusivity | ||
|
|
||
| - **OW-7**: No other workflow references `secrets.OPENAI_API_KEY`. Verifiable by `test_workflow_config.py::test_openai_key_only_in_openai_workflow` (SC-002). | ||
| - **OW-8**: `OPENAI_API_KEY` does NOT appear in the `parity-tier2.yml` file (feature 028's Claude workflow). | ||
| - **OW-9**: `ANTHROPIC_API_KEY` does NOT appear in `parity-tier2-openai.yml`. The two workflows have exclusive per-provider keys. | ||
|
|
||
| ## 5. Preflight audit | ||
|
|
||
| - **OW-10**: A preflight step MUST log actor + SHA + fixture_glob + selected model to `$GITHUB_STEP_SUMMARY` BEFORE the SDK-invocation step consumes `OPENAI_API_KEY`. | ||
|
|
||
| ## 6. Runner invocation | ||
|
|
||
| - **OW-11**: The SDK step invokes `uv run python -m tests.darnit.parity.tier2.run --backend openai --fixture-glob <glob> --model <model>`. | ||
| - **OW-12**: The step's `env:` block sets `OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}`. | ||
| - **OW-13**: `ANTHROPIC_API_KEY` is NOT set in the step's `env:` block. This is enforced by an assertion in `test_workflow_config.py` that greps `parity-tier2-openai.yml` for `ANTHROPIC_API_KEY` and asserts the count is zero. | ||
|
|
||
| ## 7. Artifact upload | ||
|
|
||
| - **OW-14**: `actions/upload-artifact@v4` runs with `if: always()` so failure artifacts land on any exit code. | ||
| - **OW-15**: Upload path is `parity-artifacts/`, same as feature 028. The OpenAI backend writes `openai_final_message.md` per fixture (distinct filename from feature 028's `skill_final_message.md`) so both providers' artifacts can coexist under the same fixture directory across dispatches. | ||
|
|
||
| ## 8. Exit codes | ||
|
|
||
| - **OW-16**: Runner exit codes for OpenAI backend match the extended set from feature 028 + feature 029: | ||
| - `0` -- success | ||
| - `1` -- per_control_disagree or counts_disagree | ||
| - `2` -- skill_unparseable | ||
| - `3` -- setup (missing `OPENAI_API_KEY`) | ||
| - `4` -- rate limit | ||
| - `5` -- turn_cap_exhausted (NEW in feature 029) | ||
|
|
||
| ## 9. Rate limit handling | ||
|
|
||
| - **OW-17**: The runner MUST NOT retry API calls automatically on rate limit. Same policy as feature 028's Claude workflow. | ||
|
|
||
| ## 10. Reviewer checklist | ||
|
|
||
| Before approving a dispatch of `parity-tier2-openai.yml`, the reviewer verifies: | ||
|
|
||
| - The dispatcher (github.actor) is listed on the workflow run and matches an authorized maintainer. | ||
| - The `fixture_glob` input matches the intended investigation scope (`"*"` for full-corpus check, a specific fixture name for targeted debugging). | ||
| - The `model` input either uses the workflow's pinned default OR is an explicitly overridden version-suffixed string. If a moving alias (e.g., `gpt-4o`) is in the model input, decline the approval and instruct the dispatcher to specify a versioned model. |
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.
Uh oh!
There was an error while loading. Please reload this page.
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: Move all github context and workflow inputs to env: variables before using them in the run: block to prevent shell injection. Replace direct ${{ }} interpolation with shell environment variable references.
Recommended Code Changes: