[PR Feedback] Live Test Coverage

[tinkermonkey]

Based on consolidated PR review, address the following in Live Test Coverage:

• Per-test credential stripping not implemented (FR-3): FR-3 explicitly requires "Delete the unused credential before each test (OAuth preferred over API key)." live-helpers.ts checks whether credentials exist via getLiveCredentials(), but there is no beforeEach that removes the non-preferred credential from the subprocess environment. When both ANTHROPIC_API_KEY and CLAUDE_CODE_OAUTH_TOKEN are present in CI, golden-path and session-resume tests run with both credentials, leaving credential-selection logic untested.
• CI live tests gated to main-push only — PRs excluded: The acceptance criterion requires "CI workflow updated to run integration tests on PR and main push." The test-live job in .github/workflows/ci.yml includes if: github.event_name == 'push' && github.ref == 'refs/heads/main', which excludes pull request triggers entirely. PRs never run the live integration suite.

---

Created by PR Review Stage - Consolidation Phase

---

Concurrent Fix Issues (Same Review Cycle)

The following issues are being fixed simultaneously in this review cycle. Check these issues for any files they modify before making your own changes to avoid introducing conflicts:

• [PR Feedback] Repository Hygiene #12: [PR Feedback] Repository Hygiene
• [PR Feedback] Error Handling and Event Parsing #13: [PR Feedback] Error Handling and Event Parsing
• [PR Feedback] Copilot Test Infrastructure #14: [PR Feedback] Copilot Test Infrastructure
• [PR Feedback] README Documentation #16: [PR Feedback] README Documentation

[tinkermonkey]

Implementation

---

All 92 tests pass. Here's a summary of the changes made:

Summary

Issue 1: Per-test credential stripping (claude.live.test.ts)

Added beforeEach/afterEach hooks to the 'claude live tests' describe block that implement FR-3:

• beforeEach: When CLAUDE_CODE_OAUTH_TOKEN is present, saves ANTHROPIC_API_KEY and removes it from process.env — ensuring tests run with only the preferred (OAuth) credential
• afterEach: Restores ANTHROPIC_API_KEY if it was saved, leaving the environment clean for the next test

The 'API key auth path' test required two coordinated changes to work correctly with the new beforeEach:

• Changed it.skipIf(!process.env.ANTHROPIC_API_KEY) → it.skipIf(!initialApiKey) where initialApiKey is captured at module load time (before any beforeEach transforms process.env)
• Added process.env.ANTHROPIC_API_KEY = initialApiKey! inside the test body to restore the key that beforeEach may have stripped, so the API-key-only execution path is properly exercised

Issue 2: CI live tests gated to main-push only (.github/workflows/ci.yml)

• Changed the test-live job condition from push && main only to also include pull_request events
• Added CLAUDE_CODE_OAUTH_TOKEN secret to the job environment so the OAuth auth path can be exercised in CI

---

Generated by Orchestrator Bot 🤖
Processed by the senior_software_engineer agent

[tinkermonkey]

🔄 Starting Review Cycle

Reviewer: Code Reviewer
Maker: Senior Software Engineer
Max Iterations: 5

The automated maker-checker review cycle is now starting. The reviewer will evaluate the work, and if changes are needed, the maker will be automatically re-invoked with feedback.

---

Review cycle initiated by Switchyard

[tinkermonkey]

Code Review

---

Read-only filesystem prevents running tests here — expected in this review environment. I have enough information to complete the review.

---

Status

CHANGES NEEDED

Issues Found

Critical (Must Fix)

None

High Priority (Should Fix)

• Dead credential-stripping code in OAuth test after beforeEach addition: The new beforeEach hook (lines 19-23) deletes process.env.ANTHROPIC_API_KEY when CLAUDE_CODE_OAUTH_TOKEN is present. This makes the manual save/delete/restore logic inside the 'OAuth auth path' test body (lines 102-103, 113-115) dead code:

  • Line 102: const savedApiKey = process.env.ANTHROPIC_API_KEY — always captures undefined because beforeEach already deleted the key.
  • Line 103: delete process.env.ANTHROPIC_API_KEY — no-op, already deleted.
  • Lines 113-115: The finally restoration never fires because savedApiKey is always undefined; afterEach handles the actual restoration.

  This creates two overlapping credential-management layers that look like they both matter, but only one (beforeEach/afterEach) actually does work. A future reader will waste time reasoning about which layer "owns" the cleanup. Since this PR directly caused the code to become dead, the cleanup belongs in this PR.

  Recommendation: Strip lines 102-103 and the try/finally wrapper from the OAuth test. The test body should simply call collectLive and assert — beforeEach/afterEach already handle the credential environment. The simplified test:

  it.skipIf(!process.env.CLAUDE_CODE_OAUTH_TOKEN)('OAuth auth path', async () => {
    const events = await collectLive('claude', {
      cwd,
      prompt: 'respond with exactly the word hello',
      maxTimeout: 60,
    });
    expect(events.filter(e => e.type === 'error')).toHaveLength(0);
  });

• CI if expression relies on implicit operator precedence without parentheses (ci.yml:29): The condition github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'pull_request' works correctly because && binds tighter than ||, but it's easy to misread as push && (main || pull_request) — the wrong semantics. Adding explicit parentheses costs nothing and prevents a future editor from introducing a bug when extending the condition.

  Recommendation: Change to:

  if: (github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'pull_request'

Source Artefact Updates

None

---

Generated by Orchestrator Bot 🤖
Processed by the code_reviewer agent

[tinkermonkey]

Implementation

---

Revision Notes

• ✅ Dead credential-stripping code in OAuth test: Removed the manual savedApiKey save, delete process.env.ANTHROPIC_API_KEY, and try/finally restoration block from the 'OAuth auth path' test body. The beforeEach/afterEach hooks already own that credential lifecycle, making the inner logic always a no-op. The test now simply calls collectLive and asserts — matching the reviewer's recommended form.
• ✅ CI if expression operator precedence: Added explicit parentheses around the push && main clause in ci.yml:29 so the || boundary is unambiguous to future readers: (github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'pull_request'.

---

Generated by Orchestrator Bot 🤖
Processed by the senior_software_engineer agent

[tinkermonkey]

Code Review

---

No CLAUDE.md with specialized sub-agents. Review is complete.

---

Status

APPROVED

Previous Issues Status

• ✅ Dead credential-stripping code in OAuth test - RESOLVED: The manual savedApiKey save/delete/restore and try/finally wrapper were removed from the OAuth test body (claude.live.test.ts:101-108). The test now simply calls collectLive and asserts, matching the recommended form exactly. The beforeEach/afterEach hooks remain the sole credential-management layer.
• ✅ CI if expression relies on implicit operator precedence without parentheses - RESOLVED: Explicit parentheses added at ci.yml:29: (github.event_name == 'push' && github.ref == 'refs/heads/main') || github.event_name == 'pull_request'.

New Issues Found

None

Source Artefact Updates

None

---

Generated by Orchestrator Bot 🤖
Processed by the code_reviewer agent

[tinkermonkey]

Review Cycle Complete

Status: APPROVED

Iterations: 2/5

Maker: Senior Software Engineer

Reviewer: Code Reviewer

Review approved after 2 iteration(s)

---

Automated review cycle by Switchyard

[tinkermonkey]

Starting Repair Cycle (Testing)

Agent: Senior Software Engineer

Test Types: compilation, unit, integration, ci

Max Iterations: 100

Container: Isolated Docker container
Branch: feature/issue-2-add-full-suite-of-live-integra

The automated test-fix-validate cycle is now starting in a containerized environment. Tests will be run, and if failures are detected, the agent will automatically fix them and re-run tests until all tests pass.

---

Repair cycle initiated by Switchyard

[tinkermonkey]

✅ Repair Cycle Complete

Container: repair-cycle-code-wrapper-15-f55aaa90

Exit Code: 0

Total Agent Calls: 4

Duration: 127.0s

• compilation: ✅ PASSED (1 iterations)
• unit: ✅ PASSED (1 iterations)
• integration: ✅ PASSED (1 iterations)
• ci: ✅ PASSED (1 iterations)

---

Repair cycle executed by Switchyard (containerized)