(janitor/test-prune): remove redundant installFromSource mock-echo test in kiloclaw-router - #5671
Merged
Conversation
The 'passes through the no_instance outcome' test only asserted that installFromSource's result equals the exact object handed to a same-test mock, with no branching logic exercised in the router. installFromSource (kiloclaw-router.ts:3256-3273) is a literal pass-through to dispatchInstallFromSource; it does not inspect or transform the ok/code shape it returns. The sibling test 'dispatches for an entitled caller and returns the dispatch result' already drives the identical code path with the same input and additionally asserts the call arguments forwarded to the service, making it a strictly stronger check of the same pass-through behavior. The no_instance branch itself is business logic owned by dispatchInstallFromSource, not the router, and is already covered directly (with distinct triggering conditions) in apps/web/src/lib/kiloclaw/install-dispatch.test.ts. No coverage is lost: - Router pass-through behavior: covered by the sibling success test. - Auth gate: covered by the FORBIDDEN test in the same describe block. - Input validation: covered by the bad-source test in the same block. - no_instance business logic: covered directly in install-dispatch.test.ts.
Contributor
Author
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Executive SummaryRemoved one redundant mock-echo test from Files Reviewed (1 files)
Reviewed by grok-4.6 · Input: 33.8K · Output: 5.1K · Cached: 172.7K Review guidance: REVIEW.md from base branch |
pandemicsyn
approved these changes
Aug 28, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Removes one fully redundant test from
apps/web/src/routers/kiloclaw-router.test.ts.it('passes through the no_instance outcome', ...)in thekiloclawRouter installFromSourcedescribe block only asserted that the router's result equals the exact literal object handed to a same-test mock (installDispatchMock.__dispatchInstallFromSource.mockResolvedValue({ ok: false, code: 'no_instance' })→expect(result).toEqual({ ok: false, code: 'no_instance' })). No router logic branches on this shape.Why this is safe to delete
installFromSource(apps/web/src/routers/kiloclaw-router.ts:3256-3273) is a literal pass-through todispatchInstallFromSource; the router does not inspect or transform the{ ok, code }result it returns. Swapping the mocked literal from a success shape to ano_instanceshape and re-assertingtoEqualcannot detect any regression that the sibling test wouldn't already catch, since any bug in the pass-through (dropped field, wrapped response, broken call) would already fail the sibling's assertions.Surviving coverage for the deleted test:
it('dispatches for an entitled caller and returns the dispatch result', ...)in the same describe block, which drives the identical code path with the same input and additionally asserts the exact arguments forwarded todispatchInstallFromSource— a strictly stronger check.it('rejects a caller without active KiloClaw access (FORBIDDEN)...')in the same describe block.it('rejects an unregistered source via input validation...')in the same describe block.no_instancebusiness logic itself (owned bydispatchInstallFromSource, not the router): covered directly, with distinct triggering conditions (no active instance, null sandbox id, kilo-chatno_conversationmapping), inapps/web/src/lib/kiloclaw/install-dispatch.test.ts(lines 140, 214, 231).Why the removed test could not detect an additional plausible regression
The router performs zero branching or transformation on the mocked outcome — it returns whatever the mocked service resolves to, verbatim. The only way this test could fail is if the sibling pass-through test (with a different mocked payload) would also fail. There is no failure mode unique to the
no_instanceliteral at the router layer; that logic lives in and is tested byinstall-dispatch.test.ts.Validation
npx jest src/routers/kiloclaw-router.test.ts -t installFromSource: file parses/compiles correctly; the 3 remaining tests in this describe block are unchanged in behavior. Execution against the DB-backed suite fails in this sandbox only because Postgres isn't reachable here (no docker available), not due to this change — confirmed the same pre-existing DB connection failure occurs for the untouched sibling tests too.pnpm -w exec oxlint --config .oxlintrc.json apps/web/src/routers/kiloclaw-router.test.ts: 0 warnings, 0 errors.npx tsgo --noEmit -p tsconfig.json(apps/web): no errors.oxfmton the changed file: no formatting changes needed.git diff --check: clean.