fix(tests): unbreak tool_use integration tests merged in #3637#3656
Merged
Conversation
Build Check was cancelled when #3637 merged so these tests landed unverified. Subsequent main runs have all been red. Five issues, fixed forward (production behavior unchanged except for the next_event days rounding): - brand-classifier-route + brand-enrichment-route tests used '${pid}_${Date.now()}' as the SUFFIX, producing underscored test domains. enrichBrand rejects those via /^[a-z0-9.-]+\.[a-z]{2,}$/ → route returned 500. Switch separator to hyphen. - property-enhancement-function test mocked AdAgentsManager with vi.fn().mockImplementation(...). vi.fn returns a function, not a constructor; production calls `new AdAgentsManager()`. Replace with a real class. - prospect-triage-function test raced logTriageDecision, which is fire-and-forget by design. Add a small awaitTriageLog poll helper instead of changing the production call to await (that would alter caller-visible latency for a per-domain triage decision). - member-context.ts: next_event "in N days" rendered with Math.floor on (starts_at - now) / 86_400_000, which flipped "in 5 days" to "in 4 days" when a sub-millisecond gap accrued between the test setting starts_at and the formatter computing the diff. Switch to Math.round — also semantically correct (an event 4.7 days away should render as "in 5 days", not "in 4 days"). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
enabled auto-merge (squash)
April 30, 2026 11:24
This was referenced Apr 30, 2026
bokelley
added a commit
that referenced
this pull request
Apr 30, 2026
…#3661) Follow-up to #3656. Three integration test files still used `${process.pid}_${Date.now()}` (underscore separator) for their PID+timestamp SUFFIX: - property-enhancement-function.test.ts - brand-properties-parse.test.ts - addie-brand-property-tools.test.ts Underscores are invalid in domain names per RFC 1035. These three pass today because their TEST_DOMAIN never hits a regex-validating code path — property-enhancement skips validation; brand-properties-parse and addie-brand-property-tools mock out the routes that would. But the next test that adds a real `enrichBrand` / `expandHouse` call would rediscover the bug that #3656 just fixed. Switched to a hyphen across the board to match the convention established in #3656 (`brand-classifier-route`, `brand-enrichment-route`, `prospect-triage`). No behavior change for the existing tests; org/user IDs are still opaque strings, just with a hyphen instead of an underscore in the suffix. Verified locally: 39/39 tests pass across the three affected files. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
1 participant
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
Build Check has been red on main since #3637 merged with cancelled CI. Five issues — four broken tests + one pre-existing flake — fixed forward. All 44 tests across the affected suites pass locally.
What was broken
brand-classifier-route.test.tsSUFFIX = '${pid}_${Date.now()}'→ underscored domain →enrichBrandrejects via regex → 500brand-enrichment-route.test.tsproperty-enhancement-function.test.tsvi.fn().mockImplementation(...)is not a constructor; production callsnew AdAgentsManager()prospect-triage-function.test.tstriageEmailDomaincallslogTriageDecision(...)fire-and-forget; SELECT raced INSERTawaitTriageLogpoll helper (don't change production behavior)member-context.ts > next_eventMath.floor((starts_at - now) / 86_400_000)flipped "in 5 days" → "in 4 days" on sub-millisecond gapMath.round— also semantically correctWhy the production code change for
next_eventThe pre-existing flake is pure UX: an event 4.7 days from now should render "in 5 days", not "in 4 days". The test author set
starts_at = now + 5 daysand expected "in 5 days"; the formatter racedDate.now()and floored. Switching toMath.roundfixes both the flake and the user-visible rounding behavior.Test plan
WORKOS_API_KEY=sk_test_dummy WORKOS_CLIENT_ID=client_test_dummy DATABASE_URL=...)test:unit + test-dynamic-imports + typecheck) — greenRelated
🤖 Generated with Claude Code