Refactor oversized cross-cutting tests into slice-local and concern-named files

[gr3g-bot]

Ref: #140

Summary

• move command-owned tests from oversized shared files into the owning command slices
• replace the large cross-cutting test buckets with smaller concern-named shared test files and focused support helpers
• update TypeScript test/build config so the new Vitest-based test layout typechecks and excludes slice-local tests from build output

Testing

• npm test
• npm run lint
• npm run typecheck
• npm run build

[kl4r1554-bot]

QA Review — PR #141 / Issue #140

Decision: CHANGES REQUIRED

The structural intent of this refactor is sound. test/wrapper.test.ts and test/core.test.ts are gone, the new slice-local handler tests are well-populated, the concern-based naming under test/wrapper/ and test/core/ is correct, the test/support/ helpers are clean, and the tsconfig changes are necessary and correct. All 302 tests pass; lint, typecheck, and build are clean.

However, there are duplicate tests — both within-file duplicates and cross-file duplicates — that directly undercut this refactor's stated goal of making ownership and intent clear.

---

Blockers

1. test/wrapper/opencode-context.test.ts — identical test bodies on lines 14–32 and 34–52

"executeOrfeTool still rejects missing caller context for caller-mapped commands" (lines 34–52) is a byte-for-byte duplicate of "executeOrfeTool rejects missing caller context clearly" (lines 14–32). Same command (issue get), same empty context {}, same assert.deepEqual expectation. The second test name implies it should exercise a different scenario (a command with explicit caller-context requirements), but the implementation was never differentiated. This appears to be an incomplete test — either finish the intent (use a genuinely caller-mapped command or a different command group) or remove the duplicate.

2. test/wrapper/path-overrides.test.ts — identical test bodies on lines 60–93 and 95–127

"executeOrfeTool returns structured config failures for issue validate when config path is invalid" (lines 95–127) is a byte-for-byte duplicate of "executeOrfeTool returns structured config failures when forwarded config path is invalid" (lines 60–93). Same command (issue validate), same body string, same missingConfigPath, same assert.deepEqual. The second test name is more specific but the body is not — it looks like the intent was to demonstrate the behavior for a second command or a second failure mode, but only one scenario was actually written. Remove the duplicate or implement the distinct scenario the second test name promises.

3. test/core/auth-config-loading.test.ts — command-specific auth token tests that duplicate src/commands/auth/token/handler.test.ts

The following tests in test/core/auth-config-loading.test.ts are auth token-specific success/failure paths. Per the issue's placement guidance, command-specific success/failure paths belong in the owning slice. They are also already covered by src/commands/auth/token/handler.test.ts, creating duplication:

• "runOrfeCore mints an auth token for the resolved caller bot" (lines 10–45) — same fixture, same assertion as handler.test.ts line 9. The only mechanical difference is that this version calls runOrfeCore directly while the handler test uses runCoreCommand; the underlying fixture factories are identical.
• "runOrfeCore rejects bot override input for auth token" (lines 47–67) — same assertion as handler.test.ts line 63.
• "runOrfeCore fails clearly for auth token when the installation is missing" (lines 91–118) — auth-token-specific GitHub API failure path (installation 404). Belongs in the slice.
• "runOrfeCore fails clearly for auth token when token minting is rejected" (lines 121–149) — auth-token-specific GitHub API failure path (token 403). Belongs in the slice.

The genuinely cross-cutting tests in that file (unmapped callers, empty caller names, config loading failure ordering, config surface for auth token) can stay. The four above should move to or be cleaned up from the cross-cutting file, since they're already owned by the slice.

---

Important

4. test/core/plain-data-boundary.test.ts — single test that overlaps the first test in src/commands/issue/get/handler.test.ts

"runOrfeCore can be exercised directly with plain callerName data" (lines 8–33) exercises issue-get and asserts the same fixture result as "runOrfeCore reads an issue and returns structured success output" in handler.test.ts. The only difference is that this version calls runOrfeCore directly (via runCoreCommand helper, which itself wraps runOrfeCore with the same factories). The "plain data boundary" framing doesn't establish a clearly distinct cross-cutting concern. If the intent is to verify that runOrfeCore is directly invokable without the wrapper, that's worth a brief comment in the handler test rather than a one-test shared file whose only content duplicates a slice test. Not a hard block, but the file currently muddles rather than clarifies ownership.

---

What I verified

• PR body first line is exactly Ref: #140 ✓
• Branch is issues/orfe-140 ✓
• Old test/wrapper.test.ts and test/core.test.ts deleted ✓
• 15 slice-local handler.test.ts files added under the correct command directories ✓
• New shared files use concern-based names (opencode-context, path-overrides, runtime-routing, auth-config-loading, plain-data-boundary) ✓
• test/support/ helpers are well factored and cleanly named ✓
• tsconfig.json updated to include test/**/*.test.ts for typecheck ✓
• tsconfig.build.json updated to exclude src/**/*.test.ts from build output ✓
• No new generic catch-all file introduced ✓
• All 302 tests pass ✓
• Lint: no errors ✓
• Typecheck: no errors ✓
• Build: no errors ✓

---

Summary

The skeleton of this refactor is correct and the large-scale movement of tests into slices is solid. The blockers are all about test body duplication that slipped through — two within-file pairs in the new wrapper files and one cross-file group in auth-config-loading.test.ts. These are straightforward to fix: remove or differentiate the duplicate bodies within each file, and move the four auth token-specific tests from auth-config-loading.test.ts into the slice (or simply confirm they're redundant with what's already there and remove them from the shared file).

[kl4r1554-bot]

QA Re-review — commit f06d755

Decision: APPROVED

All three blockers from the previous QA round are resolved. Verification passes cleanly.

---

Blocker resolution confirmed

1. test/wrapper/opencode-context.test.ts — duplicate removed ✅
The identical copy "executeOrfeTool still rejects missing caller context for caller-mapped commands" is gone. File now has 4 distinct tests, no duplicates.

2. test/wrapper/path-overrides.test.ts — duplicate removed ✅
The identical copy "executeOrfeTool returns structured config failures for issue validate when config path is invalid" is gone. File now has 2 distinct tests, no duplicates.

3. test/core/auth-config-loading.test.ts — command-specific tests removed; file is now genuinely cross-cutting ✅
All four auth token-specific tests ("mints an auth token for the resolved caller bot", "rejects bot override input for auth token", installation-missing 404, token-minting 403) are removed. The remaining tests now use issue get uniformly — making unmapped-caller and auth-config-surface tests properly command-agnostic. A new test was added: "runOrfeCore forwards explicit auth config paths into shared auth config loading" — this is a real cross-cutting concern (verifying that authConfigPath and cwd reach loadAuthConfigImpl) not previously covered at this layer. Good addition.

---

Verification

• 296 tests pass (302 → 296; net −6 duplicates/misplaced tests removed) ✅
• Lint: no errors ✅
• Typecheck: no errors ✅
• Build: no errors ✅

---

Remaining non-blocking note

test/core/plain-data-boundary.test.ts (1 test) still overlaps the first test in src/commands/issue/get/handler.test.ts — noted as Important in the first round, not a blocker, and not a gate here. Worth a follow-up cleanup but does not affect acceptance.

[MirzaMerdovic]

There is a failing test that needs to be fixed.
I also don't see the point of having the support folder for most of those file because they are very specific to command sub-groups and can be moved there under a test folder or similar i.e. all *fixtures files

[kl4r1554-bot]

QA Re-review — commit 2419b54

Decision: APPROVED

All three human-requested follow-ups are correctly addressed. The branch is clean.

---

Requested changes — confirmed resolved

1. Failing test — resolved ✅
All 295 tests pass. No failures across 28 test files.

2. Fixture files moved closer to owning command groups — resolved ✅

Moves verified exactly as described:

• test/support/auth-fixtures.ts → test/auth/fixtures.ts
• test/support/issue-fixtures.ts → test/issue/fixtures.ts
• test/support/pr-fixtures.ts → test/pr/fixtures.ts
• test/support/project-fixtures.ts → test/project/fixtures.ts

All 14 slice-local handler.test.ts imports updated accordingly. No stale references to the old test/support/*-fixtures paths remain anywhere in the tree.

The three files kept in test/support/ are the right ones — command-runtime.ts, http-test.ts, and runtime-fixtures.ts are all genuinely cross-cutting (used by both shared and slice-local tests, across multiple command groups), so they belong there.

3. test/core/plain-data-boundary.test.ts removed — resolved ✅
File deleted. test/core/ now contains only the two concern-named shared files: auth-config-loading.test.ts and runtime-routing.test.ts. Test count correctly drops from 296 to 295.

---

Verification

• 295 tests pass across 28 test files ✅
• Lint: no errors ✅
• Typecheck: no errors ✅
• Build: no errors ✅
• No stale test/support/*-fixtures references: confirmed by grep ✅

[kl4r1554-bot]

QA Re-review — commit effd39e

Decision: APPROVED

---

Reported failure — confirmed fixed ✅

test/core/auth-config-loading.test.ts — "runOrfeCore rejects unmapped callers clearly for GitHub-backed commands"

The fix is correct and the reasoning holds. For GitHub-backed commands runOrfeCore loads both repo config and auth config before checking the caller-to-bot mapping (auth config is where the mapping lives). Without a loadAuthConfigImpl stub the core was falling back to the real filesystem load, finding nothing, and surfacing config_not_found before reaching the caller mapping check. Adding loadAuthConfigImpl: async () => createAuthConfig() lets auth config loading succeed so execution reaches the mapping check, where "Unknown Agent" is correctly found to be unmapped and caller_name_unmapped is raised.

The surrounding tests are each set up consistently with this ordering:

• "rejects empty caller names clearly" — omits loadAuthConfigImpl correctly because blank/whitespace caller names are caught by a pre-flight format check before any config loading runs
• "surfaces auth config loading failures clearly" — intentionally throws in loadAuthConfigImpl ✓
• "forwards explicit auth config paths" — intentionally throws in loadAuthConfigImpl after capturing forwarded options ✓
• "rejects repo-local config failures before auth config loading succeeds" — both stubs throw, ordering assertion is correct ✓

Verification

• 295 tests pass across 28 test files ✅
• Lint: no errors ✅
• Typecheck: no errors ✅
• Build: no errors ✅

No other files changed. No regressions introduced.