[IAmJSD]

Summary

Two remaining test-integrity items from #353. Other parts of that issue stay open.

1. The last five bare panic(...) sites in internal/mocks

Root cause. #1270 moved 82 of the mocks' type-assertion sites onto testReporter. Mock methods added since then went back to the old pattern: panic(fmt.Sprintf("mock: expected ...")) on a mistyped .Return(...). A panic like that ends the whole test binary. The remaining tests in the package don't run, TestMain teardown (such as a shared Postgres container) is skipped, and the output is a stack trace instead of a failure filed under the test's name. On main (7b86e38), five sites in internal/mocks/stores.go still do this:

• GetMarketplaceListingAttempt
• ListMarketplaceListingsToPoll
• ListStoredRecommendationsChunk
• ListHistoryExecutionsPage
• ListPurchaseHistoryPage

Fix. Each site now calls m.reporter.wrongReturnType(<method>, <want type>, <got>), the same as the other 82. A mock with a registered test fails that test under the method's name. Without a registered test, testReporter.fail still panics, now naming the method. Behavior for correctly typed returns is unchanged.

Guard. The new internal/mocks/bare_panic_guard_test.go (TestMocks_NoBarePanicOutsideTestReporter) parses the package's non-test files. It fails on any call to panic( outside testReporter.fail, the one allowed site, and reports each offender as file:line:col in Recv.Method (or package scope). It also checks itself: it fails if it can no longer find the allowed panic, so a broken scan can't pass vacuously. Because the five methods now call wrongReturnType, the existing TestMocks_EveryWrongReturnTypeSiteReportsItsOwnMethod discovers them automatically. It checks that each one fails the registered test under its own name instead of panicking or returning.

2. frontend/src/__tests__/api.test.ts 'includes status code in error'

The assertion sat inside a catch with no expect.assertions. If apiRequest resolved on a 500 instead of rejecting, no assertion ran and Jest reported a pass. It is now await expect(apiRequest('/test')).rejects.toMatchObject({ status: 500 }). This is the only frontend file touched, as allowed for this task.

Refs #353

Verification

All evidence below is unit, mock or fixture evidence. Nothing here touches a database or a real service.

export PATH=$HOME/.local/go/bin:$HOME/go/bin:$HOME/.local/bin:$PATH
export AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true
go build ./... && go vet ./... && go vet -tags integration ./...      # clean
go test -count=1 ./internal/mocks/                                     # ok
golangci-lint run --allow-parallel-runners ./internal/mocks/...        # 0 issues
gocyclo -over 10 -ignore "_test\.go" internal/mocks/stores.go          # none
cd frontend && npx jest src/__tests__/api.test.ts --coverage=false     # 100/100 passed
npx eslint src/__tests__/api.test.ts && npx tsc --noEmit -p .          # clean

Fail-before / pass-after (mocks).

• Before the fix, TestMocks_NoBarePanicOutsideTestReporter fails, listing exactly stores.go:962, :1013, :1816, :2097 and :2128 in the five methods above. After the fix it passes.
• After the fix, the five new TestMocks_EveryWrongReturnTypeSiteReportsItsOwnMethod/MockConfigStore.<method> subtests pass. They did not exist before, because the scan only finds wrongReturnType callers.

Mutation probes (each applied alone, then reverted) all fail the named test:

Probe	Failing test
a panic("probe") added inside MockSESClient.SendEmail	the guard, which reports ses.go:21:3 in MockSESClient.SendEmail
a package-level var _ = func() int { panic("probe") }	the guard, which reports in package scope
one of the five sites reverted to the bare panic	the guard
GetMarketplaceListingAttempt passing a neighbor's method name to wrongReturnType	the per-site test's GetMarketplaceListingAttempt subtest

Fail-before / pass-after (frontend). I temporarily changed apiRequest to return null instead of throw error on a non-OK response (the regression the issue describes), then reverted it.

• The old try/catch test passes against that regression (1 passed).
• The new rejects.toMatchObject test fails against it.
• Against the real client, the new test passes.

Combined-tree verification. This branch was tested together with the other queued branches on a local, never-pushed tree: queue/platform @ 400d256b = origin/main 7b86e386 plus these heads, merged in this order:

Branch	Tested head
fix/issue-243-rest	007c163
fix/issue-1206	312072f
fix/issue-111-email	6d422e2
fix/issue-111-fail-closed	3817e29
fix/issue-712-age-cap	a91120b
fix/issue-712-fargate	aeaf0d4
fix/issue-613-legacy-ids	350e837
fix/issue-120-guards	6f38ebc
fix/issue-120-shared-predicate	cb25735
fix/issue-151-account-update-keep	d20cf01
fix/issue-713-upcoming-filter	e73fdfb
fix/issue-263-bastion-opts	7861f32
fix/issue-43-fixture-ids	8cee203
fix/issue-177-makefile-walk	365a9d6
fix/issue-27-seller-not-registered	d845f66
fix/issue-313-user-columns	6d759dd
fix/issue-353-mock-panics	aac2edf
fix/issue-8-config-stamps	b548ae5
fix/issue-30-monthly-totals	f637921

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers. The full suite ran on f7c48020, which differs from this tree only by test-only commits on fix/issue-30, fix/issue-713 and fix/issue-263 that replace placeholder account-number literals with named constants:

• Passed: go build ./..., go vet ./..., go vet -tags integration ./... and the full go test -race -short -count=1 ./....
• Static checks: golangci-lint v2.10.1 found 0 issues on every changed package, with and without --build-tags integration. gocyclo -over 10 is silent on the changed non-test files.
• Integration tests, every changed package that has them, one package at a time (-tags integration -count=1): internal/analytics, internal/api, internal/auth, internal/config, internal/database/postgres/migrations, internal/purchase, internal/scheduler and internal/server all pass. Migrations 000149, 000150 and 000151 apply in order on this tree.
• Frontend: the changed jest files (approval-details, settings-accounts, api) pass (221 tests).
• Secrets: the first run flagged 9 new fixture lines (placeholder account numbers next to an account key); fixed with named constants. Re-run on this tree: git-secrets, using the repo's scripts/setup-git-secrets.sh patterns in a fresh repo with an isolated HOME, found nothing in the changed files, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.
• Terraform .tftest.hcl files were not run locally (CI runs them). Nothing was applied to any cloud.

Out of scope (deliberately left for later)

• Registering t at the roughly 1,140 MockConfigStore constructions, and making the isExpected defaults opt-in. The owner question on chore(test): medium/low findings from the 2026-07-28 full review [platform part] #353 is still pending.
• Everything else still open on chore(test): medium/low findings from the 2026-07-28 full review [platform part] #353; this branch touches only the two items above.

Labels

Maintainer: please mirror the issue's triage labels onto this PR: impact/internal, type/chore, triaged, priority/p3, severity/low, urgency/eventually, effort/m.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE 66150ca525bf1d90cb5b1bc2ab7d859cdb23cb6d

Head moved from aac2edf3 to 66150ca5 during review. The only new commit bumps golang.org/x/net to v0.62.0, the same change as #1564. I re-checked at the new head; the verdict is unchanged.

Blocking:

• none

Non-blocking:

• bare_panic_guard_test.go only matches the panic builtin by identifier. It won't catch log.Panicf, t.Fatal from a non-test goroutine, or a panic in a helper outside internal/mocks. That's fine for chore(test): medium/low findings from the 2026-07-28 full review [platform part] #353's scope.
• After wrongReturnType reports, the methods keep going with zero values (ListPurchaseHistoryPage ranges over a nil v). When a test is registered this doesn't matter, because fatalf calls FailNow and ends the test. It's the same as the other 82 sites.

Attacked:

• All five sites now pass their own method name and the expected type. The guard's self-check requires that it find the allowed testReporter.fail panic, so it can't pass vacuously. The jest change asserts rejection directly.
• Ran go build ./..., go vet with and without -tags integration, go test -race -short -count=1 ./internal/mocks/ (PASS at both heads) and golangci-lint on internal/mocks (0 issues). After npm ci in a scratch worktree, npx jest src/__tests__/api.test.ts: 100 passed.
• Fail-before (Go): with stores.go reset to main, TestMocks_NoBarePanicOutsideTestReporter fails and lists exactly stores.go:962, 1013, 1816, 2097 and 2128.
• Fail-before (frontend): I changed apiRequest's non-OK branch from throw error to return null. The new 'includes status code in error' test fails, and the old try/catch version passes against the same mutation. Restored.

Body-stale:

• none

_{Reviewer: rG2. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at 66150ca525bf1d90cb5b1bc2ab7d859cdb23cb6d merged onto main (d05a1726), together with #1547, #1555, #1558, #1559, #1561, #1562 in that order (combined tree eea0132d, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on 66150ca5, green CI, and the combined-tree verification above. Squash-merged at exactly 66150ca525bf1d90cb5b1bc2ab7d859cdb23cb6d.