feat(mcp): MCP-first lead delivery — find_new_leads, qualify_leads, lead_job_status + seed-craft prompt - #168
Open
milstan wants to merge 52 commits into
Open
feat(mcp): MCP-first lead delivery — find_new_leads, qualify_leads, lead_job_status + seed-craft prompt#168milstan wants to merge 52 commits into
milstan wants to merge 52 commits into
Conversation
…ualify_leads, lead_job_status
Three composites over the new job endpoints (POST /1.6/mcp/search,
POST /1.6/mcp/qualify, GET /1.6/mcp/jobs/{id}): one ask -> net-new
qualified contactable leads; batch qualify + right-contact on known refs;
shared cumulative job poll with cursor paging. Submit+poll built into the
tools (wait_seconds window, still_running handoff), request_id retry
contract, dry_run cost gate, no-silent-spend consent copy.
The leadbay_new_leads prompt + heuristics/example-lead-craft snippet carry
the fake-leads seed discipline (registry-style buyer description, no
invented brand name, no event language, filters for hard bounds) —
validated live on staging: same ICP delivered 0/5 junk from a naive query
vs 5/5 on-profile from a crafted description-only seed.
Cross-routing anti-triggers on pull_leads / extend_lens /
bulk_qualify_leads; WORKFLOWS.md rows 46-48 + contracts; persona and
naming/deprecation doc in docs/mcp-first-lead-delivery.md.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Never combine a lowered qualification floor with channels — that buys emails for leads the AI just scored as junk (measured: 633c for a floor-scored consultancy's email on staging). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46f60b799f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
[Claude]: Session summary for Milan — my chat replies were not rendering in the Conductor thread (confirmed absent, not collapsed), so the state of record lives here:
Only open action requiring Milan: the az command in #3927. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ac5429c92
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A single qualify+channels pass buys emails for leads the same job disqualifies (measured: ~a third of enrichment spend on staging). Teach verdicts-first, channels-second as the default; backend gate tracked in product#3938. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 0.26.0 bump merged from main left the @leadbay/mcp@0.25 pin behind; the server-json-version audit rightly failed. Pin follows the version line. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ad2b36e90
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Counter-mission: MCP-first delivery exists to eliminate client-side chaining; teaching agents to orchestrate two passes reintroduces it. The spend-gating belongs in the backend (product#3938). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dbabf3cf4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…exclusion/location/fit guidance
From the live seed-interpretation eval (2/2 cold agents): tolerate the
agent-invented filters.employees:{min,max} by mapping onto the flat wire
keys; name the exact filter keys in the craft rules; never a country in
locations (France silently matched the village of Francs); exclusions are
enforced by qualification via IBP anti-patterns (leadbay_refine_prompt for
durable ones), not by the free match; rendering gains weak-batch honesty
(best fit <30), geo fence checks, and skepticism for exemplar-bridged FR
fit scores.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
github-actions
Bot
added
the
needs-manual-rebase
Contributor
|
Couldn't auto-sync with main — this PR has a merge conflict outside the version files. main shipped |
4/4 live E2E agents passed a country label despite the guidance; the backend silently fences it to a same-named town (France→Francs, United States→Statesboro — product#3939). Named client-side rejection with the omit-locations hint; zero spend on a doomed job. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…arch-qualify # Conflicts: # WORKFLOWS.md # packages/mcp/server.json
Contributor
Author
|
[Claude]: @ArtyETH06 review requested — context + what to test. Purpose of this PR: expose the backend MCP-first job endpoints ( Please TEST it, don't just read it — Milan's explicit ask. Suggested pass (staging, any test account, e.g. milstan@leadbay.ai / usual staging password):
Known staging caveats (not this PR): country-level geo resolution product#3939, US org IBP/questions mismatch, education segment uncrawled. Full E2E acceptance evidence: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80c9fd7dce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…t delivery Review fixes on top of milstan/expose-mcp-search-qualify (PR #168). Not a replacement for that PR — the delivery tools are Milan's; these are the defects found reviewing it. - WORKFLOWS.md: resolve the committed merge-conflict markers. main's friction rows keep 46/47; the delivery rows renumber to 48/49/50. Both contract sets retained. The workflows + contract-schema audits parse this file, so the markers were a hard audit failure. - qualify_leads: enforce the spend gate in code. `qualify` defaults to TRUE backend-side (~94 cost_cents/lead) and the schema had no required params, so `{lead_refs: [...500]}` alone submitted a paid job nobody approved. A paid call now needs `confirm: true`; without it the submit is withheld and a real dry-run quote is returned (`mode: "needs_confirmation"`). `confirm: false` is a veto with no round-trip; fully free calls (qualify:false, no channels) pass straight through. Mirrors the existing enrich-titles consent gate (product#3848). - qualify_leads: also return {leads, skipped} via splitItems. Its own RENDER block reads leads[]/skipped[] and both sibling tools split; returning only items[] left an agent with two empty tables. items[] is preserved for input-order per-ref mapping. - lead_job_status: forward since/limit through the block-waiting path. waitForJob dropped them, silently turning an incremental poll into a full re-read of already-seen items. - waitForJob: bound each sleep by the remaining wait so a short wait_seconds can't block for a whole 4s interval. - Currency: cost_cents renders as $ not € (the rest of the repo, including quota-windows and account_status, denominates the same ledger in dollars). - Export findNewLeads/qualifyLeads/leadJobStatus from the core entry point. - leadbay_new_leads prompt: the paid-quote step listed min_ai_score among the flags then routed to qualify_leads, which has no such property and additionalProperties:false — a literal follow was a hard schema rejection. Flags are now split per tool, and the confirm:true requirement documented. One existing test (qualify-leads.test.ts happy path) gains `confirm: true`: it purchases an email channel, so the gate withholds it. That edit is deliberate and approved — it asserted the behaviour this commit removes. pnpm -r build, -r test (1301 passed), -r typecheck all green. Co-Authored-By: Claude <noreply@anthropic.com>
ArtyETH06
added
feature
and removed
needs-manual-rebase
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b866a4290b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- collectJobSnapshot: derive the page bound from page SIZE, not a flat 20.
At limit=5 a 500-item job returned the first 100 items while reporting
done:true.
- collectJobSnapshot: keep the last non-empty page's cursor when the drain
page comes back empty; next_since was being overwritten with null, so a
caller lost its place and re-read everything it had already seen.
- rejectCountryLocations: normalize instead of exact-matching. "the United
States", "U.S", "les États-Unis" and "République Française" all passed
through to the silent same-named-town fencing this guard exists to stop.
Île-de-France and other legitimate places still pass.
- find_new_leads / qualify_leads: short-circuit LEADBAY_MOCK submits. The
mock envelope has no job_id, so the offline dry-run mode polled
/mcp/jobs/undefined and died on a missing fixture.
- list_locations is now always exposed, like list_sectors. The delivery
tools document it as the recovery path after a 400 on an unresolvable
location, which only works if it's reachable without ADVANCED=1.
- leadbay_new_leads prompt: handle still_running BEFORE judging the seed
(Phase 3 had no such branch, so a live partial read as "empty" and
triggered a relaunch that burns an active-job slot); require an archetype
component in request_id so two archetype searches don't dedupe to one job;
route unsupported negative constraints ("no franchises") into the seed +
a Phase 5 drop-and-say instead of silently losing them; permit the
fictional seed explicitly against the no-fabrication law; take the need
from the triggering message so the skill never asks the user to repeat
what they just said.
- lead-delivery-table: don't invent the "of the Y asked" denominator when
items_requested is absent from a job snapshot.
Budget: trimmed find-new-leads template body — the additions above pushed
it to 17188 (cap 17000). Now 16948, in line with the other large tools
rather than the outlier.
pnpm -r build, -r test (1327 passed), -r typecheck green.
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 515af8bb0b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Second Codex pass, on the review fixes themselves. - find_new_leads had the same paid surface as qualify_leads (qualify:true and/or channels) but no gate — only qualify_leads got one. Same withhold: a paid search now needs confirm:true, otherwise nothing is submitted and a real dry-run quote comes back as mode:"needs_confirmation". The trigger differs by design: qualify defaults to FALSE here, so the default search stays free and ungated. - qualify_leads is destructiveHint:true. It bills for fresh qualification and channel reveals, and hosts key approval prompts off that flag — bulk_qualify_leads already sets it for the same reason. find_new_leads stays false with a comment saying why: its default really is free. - leadbay_new_leads prompt: on a short preview, qualify what was delivered AND search only the SHORTFALL under a NEW request_id. Reusing the preview's id dedupes the paid submit into the free job; keeping the original count buys a second full batch, since novelty:org already excludes the preview deliveries. Budget: the gate copy pushed find_new_leads to 17140 (cap 17000). Trimmed the template body — dropped param prose already carried by inputSchema descriptions and condensed the intro. Now 16961. pnpm -r build, -r test (1332 passed), -r typecheck green. Co-Authored-By: Claude <noreply@anthropic.com>
…mpotency fixes Third Codex pass. - Release gate (the P1): the three delivery tools are now hidden unless LEADBAY_MCP_LEAD_DELIVERY=1. /1.6/mcp/* is live on staging only and 404s on production, so shipping them unconditionally would hand every user tools that fail on the first call. Remove the gate in the release that follows the backend deploy. mcpFirstDeliveryAllTools keeps them visible to the WORKFLOWS/routing audits, so a rollout flag never reads as "this tool doesn't exist". - find_new_leads returned summary.requested while the shared renderer reads summary.items_requested — renamed, sourced from submit.items_requested, so the "delivered X of the Y asked" line stops being unrenderable. - Both submit tools now hand snapshot.next_since forward in next_poll, so a follow-up leadbay_lead_job_status continues incrementally instead of re-reading and re-rendering rows already shown. - find_new_leads is destructiveHint:true. Annotations are static and must describe the worst case (qualify:true / channels bill, and deliveries claim org novelty), not the free default. The free path is protected by the confirm gate in execute(), not by the annotation. - qualify_leads derives a stable request_id for a consented paid submit when the caller omits one (refs + paid flags + UTC day, FNV-1a). Without it a timeout or agent retry re-charged qualification and channel purchases for the same refs. The response echoes the key actually sent. One existing assertion relaxed: find-new-leads.test.ts pinned next_poll with toEqual, which the new `since` cursor breaks; now toMatchObject, so it asserts the handoff fields without freezing the shape. pnpm -r build, -r test (1337 passed), -r typecheck green. Co-Authored-By: Claude <noreply@anthropic.com>
…dify it Fourth and final instance on this PR. routing-block.test.ts had the three delivery tools appended to TOOLS_WITH_ROUTING plus a widened ALL_TOOLS. Restored byte-for-byte; lead-delivery-routing-block.test.ts now asserts the same contract for those three — WHEN TO USE inside the 600-char window, trigger phrases, memory pointer, >=2 positive AND >=2 negative examples. The ALL_TOOLS hunk turned out to be unnecessary anyway: with the delivery flag now set in vitest.config.ts, the exported arrays already carry those tools. And route_to resolution needed nothing — that check iterates every registered tool rather than only the listed ones, so it covered them all along. This one was not the author's mistake. CLAUDE.md said, twice, "add the tool name to TOOLS_WITH_ROUTING in the routing audit", and AGENTS.md repeated it three bullets below the rule forbidding exactly that. Following the documented process produced the violation. Both docs now say to assert the contract in a new audit file and point at the worked example, so the next tool does not re-earn this review. With this, every test-path change on the PR is a new file — no established test is touched. Gates: 1532 tests, typecheck, prompts:check all green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a43db14ef
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…enforces ad-hoc exclusions
[P2] The workflow prompt said to pass every lead already SEEN into the
shortfall top-up's exclude_lead_ids. That parameter caps at 500, and a paid
run may examine up to exploration_cap's ceiling (min(20n, 1000)) — so on a
wide run the instruction builds a list the backend refuses outright, killing
the one call that exists to close a gap the user already paid toward.
The prompt now sends only the examined-and-REJECTED ids, which is also the
correct set: novelty:'org' already excludes prior deliveries, so delivered
ids were the redundant half of the list all along. Dropping them is normally
enough to fit under 500; past that, most recent 500.
Belt as well as braces: rejectOversizedExclusions refuses an over-long list
in code with the bounding rule attached, so the agent gets an actionable
error instead of an opaque 400 arriving after the spend. It counts what
would actually be SENT — canonicalIdSet dedupes, so a merely-repetitive list
is not an overflow.
[P2] The description claimed ad-hoc exclusions ("no chains") are "enforced by
QUALIFICATION". They are not. qualify scores against the org's FROZEN
questions and IBP, which need not mention chains, and the positive inverse in
the seed only shifts ranking — so a violator can survive min_ai_score, be
paid for, and be delivered. Now described honestly: no tier enforces it,
post-filter before rendering, and leadbay_refine_prompt is what makes it
durable. Trimmed to stay inside the 17000-char budget (16964).
One correction to my own test while writing it: normalizeUuid passes any
non-empty string through and only lowercases real uuids, so a malformed id
still counts toward the cap. Asserting it was free would have let a list the
backend refuses slip past the guard.
Gates: 1544 tests, typecheck, prompts:check all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb38867ce4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ortable poll, long-runners [P2] The cap guard counted canonicalIdSet(exclude_lead_ids) while the submit body posted the raw array. A 600-entry list that dedupes to 400 therefore cleared the local guard and was refused by the backend anyway — on the top-up call the user had already paid toward. My own regression from last round. The body now posts the canonical list, which is also what the idempotency key was derived from, so all three finally agree. Kept undefined when absent so compactBody drops the key instead of sending []. [P2] The country-guard exemptions were process-wide. Georgia is a US state and must survive there, but on a FRANCE account it can only mean the country; the French overseas regions are the exact mirror. A global set gave each a free pass on the wrong side and let it reach the backend. Exemptions are now keyed by client.region. Unknown region falls back to the union, deliberately: without a region we cannot tell a state fence from a foreign country, and wrongly REJECTING a correct search is the louder failure. [P2] The poll slept on a bare setTimeout(4000) and only re-checked ctx.signal.aborted after it fired, so a cancel mid-sleep waited out the full interval — twice the "exits within <=2 seconds" bound the server advertises in its own instructions. sleepUnlessAborted races the timer against the signal and removes its listener either way, so a 180s wait loop cannot accumulate one listener per poll and trip Node's warning. [P2] The longRunners list still named only the legacy bulk tools, so nothing told clients to send a progressToken for calls that block-poll 45-180s: the per-poll ctx.progress was normally absent and the call looked frozen. All three added, behind the same .filter(has) that keeps the iter-12 invariant — a deployment without the delivery flag never sees them named. Gates: 1560 tests, typecheck, prompts:check all green. Every test-path change on the PR is still a new file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc4d41db0a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
[P1] 36 chars under the 17000 cap is not a margin — any shared snippet edit would have broken tool-description-source.test.ts for whoever made it, not for us. Now 639. Trimmed wording, not rules. Every rule the description carried it still carries: - heuristics/example-lead-craft (exclusive to this tool): compressed the intro and rules 1/3/4/5/7/8, and cut rule 6's country digression down to a clause — that rule is now enforced in code by rejectCountryLocations, which returns a better-targeted message than the prose could. - The body: tightened the opening paragraph and folded "free preview first" into the free-vs-paid paragraph it was restating. - rendering/lead-delivery-table (shared with qualify_leads and lead_job_status): compressed the funnel/zero-delivered/weak-batch/ sanity-check prose. Both other tools keep 3000+ chars of headroom. One correction to the finding's premise, worth recording because it points at real work: this was never the tightest description in the repo, and it is now 4th. On main today, untouched by this PR: leadbay_prepare_outreach 16993 headroom 7 leadbay_research_lead_by_id 16973 headroom 27 leadbay_pull_followups 16948 headroom 52 So the fragility Codex describes is real but pre-existing, and 90x worse on prepare_outreach than on ours. A shared-snippet edit breaks those three first. The audit also has no early warning — it passes at 16999 and fails at 17001 — so nobody sees it coming. Both belong in their own PR. Gates: 1560 tests, typecheck, prompts:check all green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de0abe69d8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…fy key, cancel guidance
[P1] Exposed tools routed at gated ones. On a default deployment
(LEADBAY_MCP_LEAD_DELIVERY unset) leadbay_pull_leads stays exposed while
leadbay_find_new_leads is absent from tools/list — but tool descriptions are
static strings, not filtered against the exposed set, so pull_leads,
extend_lens and bulk_qualify_leads still pointed net-new asks at tools the
agent cannot see. Ordinary "find me 10 gyms in Dallas" requests routed into a
void until the flag is set.
Fixed by making the description self-aware rather than deleting the routes:
snippets/gates/delivery-release-gate tells the agent to check its own tool
list first and say net-new search isn't enabled when the target is missing.
That keeps the routing useful once the flag IS on, which deleting would have
thrown away. Kept to 391 chars — pull_leads is the tight one and still holds
467 of headroom.
[P2] The page drain was sized for qualify only. MAX_JOB_ITEMS was 500 (a
qualify job's ref ceiling), but a SEARCH may examine exploration_cap's ceiling
of min(20n, 1000) and emit an outcome for each. At limit=1 the drain stopped
at 501 pages and still returned a terminal snapshot with no cursor, so paid
deliveries — and the rejected ids the top-up needs for exclude_lead_ids —
silently never reached the render. Now 1000.
[P2] qualify-leads hashed an omitted title_gate as null while the backend
applies `prefer` when contact_titles are present. An approval that omitted the
field and a retry that passed the materialized default derived different
qualify-auto-* keys, so the retry escaped dedupe into a second paid
qualification and channel purchase. Mirrors find-new-leads.ts, which already
canonicalized it — the two tools now agree.
[P2] My own regression from last round: adding the delivery tools to
longRunners put them in the CANCELLATION sentence too, which promises a
bulk-store transition to 'cancelled' and BULK_CANCELLED on later polls. Those
jobs own no bulk-store record — cancelling stops our wait while the backend
job keeps running and keeps spending. Telling the agent to stop polling was
the worst possible advice there. The two families now get separate clauses;
delivery keeps its place in the progress list.
One correction to my own test: it banned the substring BULK_CANCELLED, but the
honest text mentions it in a negation ("there is no BULK_CANCELLED"). It now
bans the PROMISE and requires the negation. Its clause slicing also anchored
on bullet (3), which is itself conditional — it leaked the rest of the
instructions in when elicitation was not exposed.
Gates: 1572 tests, typecheck, prompts:check all green. Every test-path change
on this PR is still a new file.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db921e5b11
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…ncy, expired jobs [P2] `lead_refs: [null]` threw a raw TypeError before the spend gate. normalizeLeadRefs deliberately passes non-strings through untouched so object refs survive, so a null reached derivedRequestId and died on the first property access — a stack trace where the contract promises a named error, and past the point where a quote could be returned. rejectMalformedLeadRefs now refuses null / numbers / arrays with INVALID_LEAD_REF naming the offending indexes, after the string reshape (so the bare-string shorthand still works) and before both the spend gate and key derivation. Refusing rather than dropping is deliberate: silently filtering bad entries would qualify and BILL a subset of the batch the user listed without saying so. [P2] The NEXT STEPS table treated `stop_reason: quota` and `max_cost` as one row and offered "raise the cap and continue" for both. Raising max_cost cannot clear an org quota — the re-run burns a submit and a rate-limit slot to stop in exactly the same place. Split: max_cost gets the higher cap under a new request_id; quota gets account_status then the top-up flow, and an explicit "never a re-run". [P2] The same row hard-coded "$X" while the delivery renderer had just been taught that a France job bills in euros. Now deferred to the funnel-line currency rule. [P2] lead_job_status had no `expired` branch. It is a terminal state, and the tool description already documents the recovery (items are no longer listed; re-read the billed leads via qualify_leads.prior_deliveries) — but the table told the agent to render the delivery, so an expired job surfaced as an empty result. Added the row, and the header now says pick the ONE row matching the state instead of claiming exactly two. delivery-next-steps-contract.test.ts pins all of it against the GENERATED descriptions rather than the snippets, so a template that stops being included is caught too. Headroom: the new quota row cost find_new_leads 435 chars, which walked back last round's fix. Trimmed the currency enumeration (it duplicates the shared funnel-line rule) and two verbose cells to land at 356. Gates: 1583 tests, typecheck, prompts:check all green. Every test-path change on this PR is still a new file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73d999166a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
[P2] My previous guard checked that each lead_refs entry was an object and
stopped there. `{website: 123}` cleared it and died one level in: normalizeDomain
returns null for a non-string, so the `??` fallback ran `123.trim()` — the same
raw TypeError, in the same place, still before the spend gate could return its
quote.
It was not only `website`. Enumerated every ref read in derivedRequestId
instead of patching the one named:
normalizeUuid(r.lead_id) safe — typeof check inside
normalizeUuid(r.contact_id) safe — same
r.website ... .trim() UNSAFE via the ?? fallback
r.name?.trim() UNSAFE — ?. guards null/undefined, not a number
r.location?.trim() UNSAFE — same
Checked the sibling too: find-new-leads.ts has no unguarded .trim() at all —
its ids and labels go through canonicalIdSet / canonicalLabelSet, both
typeof-filtered. The class is confined to this file.
Two layers:
1. rejectMalformedLeadRefs now validates every declared field's TYPE and names
each offender as `index.field (type)`. The field list is
`satisfies keyof` the ref shape, so adding a field to lead_refs without
adding it here fails the typecheck rather than silently reopening the hole.
2. Key derivation no longer holds the crash primitive: a `text()` helper folds
a non-string to null instead of throwing on .trim().
Layer 2 is NOT covered by a test, and I am not claiming it is: derivedRequestId
is module-private with exactly one call site, and the guard runs before it, so
there is no reachable path to exercise it. It is there so the derivation cannot
become the crash site again if a future caller skips the guard — exporting a
private function purely to test that would be worse than the gap.
One behaviour change worth recording: `text()` folds an empty-string field to
null, where the old code kept "". `{name: ""}` and `{}` now hash alike. That is
more correct — an empty name is no name — and safe today because the feature is
unreleased, so no live keys exist under the old scheme.
Also updated qualify-malformed-refs.test.ts, which this PR added last round, to
match the richer message format. Confirmed against origin/main that the file is
not pre-existing before editing it.
Gates: 1589 tests, typecheck, prompts:check all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0da75c62ec
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
[P1] My previous fix was placed where it could not be read. Measured on the
generated description: pull_leads names leadbay_find_new_leads at char 302 —
inside the ~600-char window every host loads — while the availability caveat I
appended sat at char 16,146. A truncating host saw the route and never the
condition, so the exact deployment the caveat existed for kept routing net-new
asks at a tool absent from tools/list. It was decorative.
Fixed at the generator instead of in prose: anti_triggers take an optional
`gated: true`, and emitRoutingBlock renders "(only if listed)" beside the
target — inside the block, at char ~300. Marked the three cross-routes
(pull_leads x2, extend_lens x1) and the bulk_qualify_leads prose mention, which
had the same defect at char 1,690 and was not in the finding.
The route_to audit is unaffected: its regex matches the backticked name and
ignores trailing text.
[P1] Deleting the 391-char snippet is also the headroom fix. pull_leads goes
16,533 -> 16,176, headroom 467 -> 824, back under the ~16k target the finding
cited. The in-header marker costs 18 chars per gated route instead.
[P2] Cancellation now reaches the in-flight request. Making the sleep abortable
last round was half the job: waitForJob still checked ctx.signal only AFTER
awaiting collectJobSnapshot, and that GET took no signal at all, so a cancel
during a slow /mcp/jobs response stayed blocked past the advertised <=2s exit.
- LeadbayClient.request accepts an AbortSignal and passes it to
https.request, which aborts the socket rather than leaking it.
- The 401 retry is skipped when already aborted — its 250ms backoff would
only lengthen a wait nobody is listening to.
- collectJobSnapshot forwards the signal and stops paging on abort.
- waitForJob checks BEFORE the first poll and throws REQUEST_CANCELLED
rather than opening a request; a cancel mid-loop returns the last good
snapshot instead of surfacing an AbortError, because the user cancelled
the WAIT and the job keeps running backend-side.
Two of my own slips while doing this, both caught before commit: I first
patched the httpsRequestWithRetry call inside requestVoid (identical code
shape, first match won) — typecheck caught it; and my drain test omitted a
terminating page, which was my fixture, not the code.
Gates: 1596 tests, typecheck, prompts:check all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4f91343ff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…le indexes [P2] exclude_lead_ids silently shrank — my own regression. Once the submit body started sending canonicalIdSet(...) instead of the raw array, a non-string entry stopped reaching the backend at all: canonicalIdSet maps it to null and filters it, and blanks go the same way. The paid search then ran WITHOUT an exclusion the caller asked for, and could re-deliver and charge for exactly the lead they were trying to skip. rejectMalformedExclusions refuses the list instead, before the cap check (counting a list still full of junk would size the cap against entries that were never going to be sent). [P2] The funnel line never converted cost_cents. The recipe said `spent C.CC` and I had just taught it to pick a currency symbol — so `spent: 165` (which is 1.65) rendered as `$165.00`, a 100x overstatement of a real charge, now with a confident symbol in front of it. The rule states the division first and covers every amount, not just the flagged line: cost.spent, estimated_cost.max and the needs_confirmation quote all carry the same unit and had no formatting rule at all. [P2] Stale ref.input_indexes on a deduped retry. The qualify key is order-insensitive by design, so a reordered retry returns the ORIGINAL job, whose indexes describe the ORIGINAL order — retry [B, A] after [A, B] and A is reported at index 0. remapInputIndexes re-points them at the current caller's lead_refs by ref identity (same website folding as the key, so a pasted URL matches a bare-domain echo), and nulls the indexes for ALL items when any one cannot be matched. A missing index is a gap; a wrong index is a false statement about which company was skipped. Only runs on duplicate submits, so the normal path is untouched, and `input_indexes_remapped` tells the caller which they got. Two things the full suite caught that I had not: - error-hints.test.ts is a PRE-EXISTING audit requiring every hint to name a concrete recovery action. My new INVALID_EXCLUDE_LEAD_ID hint failed it. Rewrote the hint to lead with "Drop or fix those entries and re-call" rather than touch the audit. - exclude-lead-ids-wire.test.ts (added by this PR two rounds ago) asserted a blank entry is silently dropped — the exact behaviour now refused. Updated it; verified against origin/main first that the file is not pre-existing. Headroom: the money rule cost find_new_leads 141 chars; trimmed it plus two verbose passages in the same shared snippet to land at 318. Gates: 1607 tests, typecheck, prompts:check all green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7037b7952
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…as prospects
[P2] The cancellation fix covered only the wait path. Threading ctx.signal
through waitForJob left the three DIRECT collectJobSnapshot calls — the
default wait_seconds:0 branch of lead_job_status, find_new_leads and
qualify_leads — passing no signal at all, so a cancelled status poll still
opened a GET and sat on it. Same defect the previous round was meant to close,
one call site over.
Fixed at the helper rather than at each site: collectJobSnapshot now throws
REQUEST_CANCELLED on an already-aborted signal, so every caller inherits the
guard instead of having to remember it, and all three sites now forward
ctx?.signal. The paid SUBMIT stays deliberately non-abortable — it may already
have committed server-side; what must not happen is polling it afterwards for
a caller who has gone.
[P2] A rejected account could be rendered as a prospect. qualify_leads
DELIVERS org-owned companies that failed qualification, carrying their
negative evidence — that is the documented contract ("here's why to skip this
account" is a deliverable) — so splitItems puts them in leads[], where the
shared recipe gives them a fit bar and a "Why it fits" column. The
`disqualified` translation only ever existed in the SKIPPED table, which these
never reach. On a paid result, with a high firmographic score, that reads as a
recommendation to call the account the evaluation just rejected.
They now get their own "Evaluated — does not fit" section, branched on
status_reason / a negative qualification ai_score.
Scoped to the tools that can actually emit one: qualify_leads and
lead_job_status. find_new_leads filters disqualified candidates out via
min_ai_score and never delivers one, so it pays nothing for a rule it cannot
hit — its headroom is unchanged at 318, while the two emitters keep 2373 and
4840.
Test-suite note, reported rather than smoothed over: one full run showed
bulk-store.test.ts and import-status.test.ts failing on 5s timeouts. Both are
files this PR never touches. They pass in isolation and passed two further
full core runs plus the final workspace run. I could not reproduce it and
cannot prove it is unrelated to the new import edge
(_mcp-job-helpers -> import-leads for normalizeDomain); flagging it rather
than calling it clean.
Gates: 1614 tests, typecheck, prompts:check all green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
…arch-qualify One conflicting file: packages/core/src/client.ts. Both sides added an optional trailing parameter to the same three functions — main's Stargate work added `timeoutMs` (a wall-clock deadline for the hosted auth probe), this branch added `signal` (caller cancellation for the job-polling loop). They are complementary, not competing: `timeoutMs` bounds how long WE are willing to wait, `signal` says the caller stopped caring. Both land in the same `error` handler, which clears the deadline either way. So the resolution keeps both, ordered `timeoutMs` then `signal` so main's positional call sites (fetchTelemetryEnabled passes the deadline in slot 5) stay correct untouched. The merge left one gap worth closing. The shared test harness drops `options.signal` on the floor, so nothing ever asserted the signal reaches the wire — and the reorder put two adjacent optional params, both `undefined` on almost every call, either side of that boundary. Added test/unit/client-signal-wire.test.ts with its own node:https double to pin it; verified it fails when the signal is dropped from the request options. Gates: build, typecheck, and two consecutive full `pnpm -r test` runs green (core 123 files, mcp 116 files / 751 tests, components 5, promptforge 5). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4250ccef8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… handle, and the wait a real bound Three P2s from the Codex round, all confirmed before fixing. 1. Cancellation stopped at the socket. `opts.signal` was only forwarded to https.request, i.e. AFTER acquireSemaphore() had returned. A delivery poll cancelled while QUEUED behind five busy slots could not observe its own abort until an unrelated request finished — against a stalled peer, well past the <=2s exit the delivery tools advertise. acquireSemaphore now takes the signal and SPLICES the waiter out of the queue on abort; tombstoning it instead would have leaked one slot per cancellation, since releaseSemaphore shifts blindly and the dead waiter would still take the ++. The 401-retry path now checks for abort BEFORE surrendering its slot, so a cancelled call never re-queues just to hand the slot straight back, and its 250ms backoff is abort-aware. The re-acquire in that `finally` is deliberately NOT abortable — the request must return holding exactly one slot or the caller's unconditional release drifts the counter permanently. 2. The path-traversal guard did not guard. encodeURIComponent does not escape `.`, so a job_id of `..` survived it verbatim and new URL() normalized /1.6/mcp/jobs/..?limit=100 down to /1.6/mcp/?limit=100 — sending the bearer token to an authenticated endpoint nobody asked for. Verified directly before fixing; `../..` and friends were already safe because their slashes DO get escaped, which is why the original comment looked right. Replaced with an allowlist of the RFC 3986 unreserved set plus an explicit dot-segment rejection: it accepts every handle the backend actually issues (UUIDs, search-auto-*/qualify-auto-* keys) while structurally excluding separators, query/fragment injection and encoded traversal. 3. wait_seconds did not bound the first read. The deadline was enforced only by the loop condition, which is not evaluated until the initial snapshot returns, so even `wait_seconds: 1` could block forever on a stalled GET. Every snapshot request now carries the REMAINING budget, capped by a per-request ceiling; zero-wait callers get the ceiling, which is what stops their single poll being unbounded too. This uses the `timeoutMs` that main's Stargate work added to the client — the two changes compose. A timeout on a LATER read returns the last good snapshot rather than discarding a live job over one slow read. A timeout on the FIRST read throws an envelope that CARRIES the job_id, so a caller who just paid for a submit does not lose the only handle to what they bought. Coverage in three new files; each fix was mutation-tested by reverting it and confirming the new tests fail (the semaphore ones fail by hanging, which is the bug's actual signature). Gates: build, typecheck, prompts:check, and two consecutive full runs — 1707 tests (core 888, mcp 751, components 47, promptforge 21). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f1629f3f47
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…and make the wait bound the whole drain Three findings, two of them fair hits on last round's fixes. 1. (P1) A confirmed PAID submit passed no signal at all, so a search or qualify POST queued behind the client's five slots still went out after the user hit Cancel — charging, and claiming novelty on leads nobody asked for any more. The previous round called the submit "deliberately non-abortable" because it may already have committed server-side. That is true only ONCE IT IS ON THE WIRE; while queued, nothing has been sent and cancelling is provably free. So `request` now has two cancellation scopes: `signal` (queue + socket, for reads) and `preSendSignal` (queue only — aborts up to dispatch, then lets an in-flight request finish rather than leaving the spend ambiguous). Both paid submits pass the latter. zero-wait-cancellation.test.ts asserted the old contract, so it is updated with the reasoning recorded rather than quietly flipped. 2. (P2) Last round's deadline was per-REQUEST, which multiplies the budget by the page count: at limit:1 over a 1000-item job, 1000 sequential GETs each entitled to the full wait_seconds, and waitForJob cannot re-check its own deadline until the whole drain returns. Now one ABSOLUTE deadline is computed per snapshot and each page gets what is LEFT; the drain stops rather than starting a page that cannot finish. That makes early exit reachable, so truncation is now REPORTED, not silent: a full last page plus a live cursor means items are a prefix, and the tools surface `items_truncated` with a rendering rule. Without it a partial drain of a completed job renders "delivered 40" over 12 rows. The rule is shared by all three delivery tools rather than scoped to the drain-heavy pair — scoping would leave find_new_leads silently misreporting the day its item count crosses a page, which is the exact defect being fixed. Its budget was paid for by trimming its own body (250 chars headroom). 3. (P2) The 401 retry's re-acquisition was left non-abortable last round, and documented as a deliberate residual because the caller's `finally` releases unconditionally. That was the wrong trade: it can still block indefinitely. Fixed properly by making slot ownership explicit — a box the caller passes, so re-acquisition can reject on abort and the `finally` releases only what is actually held. Callers that do not track ownership keep the unconditional re-acquire, which is what preserves their balance. Every fix was mutation-tested by reverting it. The first attempt at the drain test passed against its own bug (a stubbed client never advances the clock, so a per-page budget and a remaining-budget are indistinguishable) — it now moves Date.now and asserts the budgets strictly decrease. The slot-accounting mutant also survived the first pass, so a dedicated 401-retry test was added; it now fails with the exact one-slot drift (`expected 4 to be 5`). Gates: build, typecheck, prompts:check, two consecutive full runs — 1720 tests (core 898, mcp 754, components 47, promptforge 21). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f0fcd4fdb0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…at is missing 1. (P2) `timeoutMs` only started once httpsRequest ran — i.e. after the semaphore was acquired. Five slow or stalled unrelated requests therefore let a bounded call sit in the queue without limit, so even `wait_seconds: 1` could overrun its contract while every individual request looked well-behaved. The wait a caller asked for is wall-clock, not socket time, so acquireSemaphore now takes an absolute deadline, rejects a queued waiter when it expires, and splices it out; the socket then gets what is LEFT rather than a fresh allowance. The 401 RETRY is the deliberate exception: it gets a fresh window measured after the backoff. `timeoutMs` is documented as bounding a single attempt and the hosted auth probe depends on that — its 250ms 401-backoff alone outlasts a 200ms probe budget, so charging the retry against the first attempt's deadline deletes the retry instead of bounding it. auth-http-mixed-outcome- retry.test.ts (from main, untouched) is what says so: it drops to 3 host attempts instead of 4. I verified that failure is real rather than assuming it, by re-testing the shared-deadline variant against a freshly built core. 2. (P2) A search that stops at `max_cost` has delivered PART of the batch, but the NEXT STEPS row only said "raise the cap and continue" — so a 10-lead ask that delivered 6 re-requested 10 rather than 4, and because `novelty: org` excludes only DELIVERED leads, the rerun also paid to re-examine the same examined-and-rejected candidates. Both overspend while the action is presented to the user as a continuation. The row now prescribes `count` = the shortfall (`items_requested` − delivered) and `exclude_lead_ids` = the examined-but-rejected ids, with the reason named. That row cost more than find_new_leads had left, so the chars were bought back where they were genuinely redundant — verbose phrasing in the shared delivery-table snippet and the seed-craft heuristics (rules kept intact), plus its own body. Headroom 198, roughly where it started. Both fixes mutation-tested: dropping the acquisition deadline hangs the queued call (the bug's real signature), restarting the clock after acquisition lands at 501ms against a 450ms bound, and reverting the rerun row fails the new audit. Gates: build, typecheck, prompts:check, two consecutive full runs — 1729 tests (core 902, mcp 759, components 47, promptforge 21). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c70a6f059e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…er paid for Four findings, all follow-ups on my own work. 1. (P2) The 401 retry ignored the whole-call deadline. Last round I gave it a fresh window because the hosted auth probe needs one — its 250ms backoff outlasts a 200ms probe budget — but that left the implementation incoherent: queue and socket shared a total, the retry did not. The real problem was one knob asked to mean two things. Now `timeoutMs` bounds ONE attempt (what the probe needs) and `totalTimeoutMs` bounds the whole call including backoff and retry (what a job poll needs, so a transient 401 cannot buy it a second full wait_seconds). Either, both, or neither; whichever expires first wins. Job snapshots switched to totalTimeoutMs. 2. (P2) `snapshotBudget` floored a spent budget at 1s, so a final poll ran a full second past the deadline — `wait_seconds: 1` took ~2s — and any fractional wait overran on the very first snapshot. Floor removed, plus a re-check after the sleep, which can itself consume the entire remainder. 3. (P2) A truncated drain on an already-TERMINAL job was unrecoverable. `done` was true so `next_poll` was null, and `next_since` only ever existed inside next_poll — so the rendering rule I added telling the agent to fetch the rest with `since: next_since` named a cursor the response did not contain. Those rows are paid for. All three delivery tools now expose `next_since` at the top level and keep a continuation whenever truncation occurred, with suggested_wait_seconds 0 since it is a page fetch, not a wait. 4. (P2) Post-submit polling failures dropped the job handle. Once the submit returns, a backend-owned job exists and may be spending; job_id is the only route back to it. Only a TIMEOUT on the block-waiting branch preserved it — an abort, a connection reset or a 502 propagated bare, and the zero-wait branch preserved nothing. Both submit paths now go through snapshotAfterSubmit, which attaches the handle to ANY failure. All four mutation-tested. Two of my own assertions were too weak to catch their own bug and were strengthened: "budget <= 1000" cannot detect a floor that IS 1000 (now: budgets must strictly decrease), and the retry-window assertions needed the total/per-attempt split to be observable. Gates: build, typecheck, prompts:check, two consecutive full runs — 1746 tests (core 919, mcp 759, components 47, promptforge 21). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
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
2 participants
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.
Exposes the backend MCP-first job endpoints (
POST /1.6/mcp/search,POST /1.6/mcp/qualify,GET /1.6/mcp/jobs/{id}) as three composites plus theleadbay_new_leadsguided prompt. One ask → net-new qualified contactableleads, replacing the pull→select→web_fetch→poll→enrich chain.
Not mergeable yet — blocked on the backend release, not on this code. See
"Release blocker" below.
What's in
leadbay_find_new_leads(composite write): one ask → n net-new leads;free by default, paid flags behind a
dry_runquote + consent, submit+pollbuilt in (
wait_seconds),request_idretry contract,still_runninghandoff.
leadbay_qualify_leads(composite write): ≤500 refs (ids / websites /names / contact ids /
prior_deliveries), per-item verdicts, owneddisqualified leads delivered WITH their negative evidence.
leadbay_lead_job_status(composite read): cumulative snapshot, cursorpaging, optional block-wait.
leadbay_new_leadsprompt +heuristics/example-lead-craftsnippet —the backend fake-leads discipline distilled to 8 rules, plus a live-measured
one: an invented
example_lead.namepoisons matching (0/5 junk → 5/5on-profile A/B on staging, same ICP). Filed as leadbay/product#3926.
docs/mcp-first-lead-delivery.md), WORKFLOWS.md rows48–50 + contracts, routing fixtures, cross-routing anti-triggers on
pull_leads/extend_lens/bulk_qualify_leads.Review fixes (
b866a429→fff45f5e)All 34 Codex review threads are resolved (4 automated passes). Beyond the spend gate below: page
bound derived from page size (a small-page drain silently truncated at 100
items while reporting
done), cursor preserved across an empty drain page andhanded forward in
next_poll, country labels normalized (the United States,U.S,les États-Unisall reached the silent same-named-town fencing),LEADBAY_MOCKsubmits short-circuited,list_locationsexposed as thedocumented 400-recovery path, a derived
request_idso a retried paid batchcannot double-charge, and
destructiveHint: trueon both paid submitters.Spend gate now enforced in code.
qualifydefaults totruebackend-side (~94 cost_cents/lead) and the schema had no
requiredarray, so{lead_refs: [...500]}alone submitted a paid job with no quote and norequest_idto dedupe a retry. A paid call now needsconfirm: true;otherwise the submit is withheld and a real backend quote returns as
mode: "needs_confirmation".confirm: falsevetoes with no round-trip;free calls (
qualify: false, no channels) pass through untouched. Mirrorsthe
enrich-titlesconsent gate (product#3848).Driven through the compiled composite, with a client that throws if a paid
POST reaches the wire:
WORKFLOWS.mdmerge conflict resolved. Literal conflict markers werecommitted at lines 60–67 and 1025–1120, and
main's friction rows collidedon numbers 46/47. Both sets retained: friction keeps 46/47, delivery
renumbers to 48/49/50. The workflows + contract-schema audits parse this
file, so this was a hard audit failure.
Prompt↔code drift fixed. The paid-quote step listed
min_ai_scorethenrouted to
leadbay_qualify_leads, which has no such property andadditionalProperties: false— a literal follow was a hard schema rejection.Flags are now split per tool.
qualify_leadsalso returns{leads, skipped}viasplitItems— its RENDERblock reads those keys and both sibling tools split; returning only
items[]left an agent with two empty tables.
items[]kept for per-ref mapping.lead_job_statusforwardssince/limitthrough the block-waiting path(
waitForJobdropped them, silently re-reading already-seen leads).waitForJobbounds each sleep by the remaining wait.cost_centsrenders as$, not€— the rest of the repo denominates thesame ledger in dollars.
findNewLeads/qualifyLeads/leadJobStatusexported from the core entrypoint.
Release gate
POST /1.6/mcp/searchreturns 404 on production (api-usandapi-fr,confirmed with a valid authenticated prod token) and 401 on staging — the
endpoints exist only on staging.
So the three tools are gated off by default: they are only registered when
LEADBAY_MCP_LEAD_DELIVERY=1. Merging this PR therefore ships nothing to usersuntil the backend lands — at which point the gate (and the flag) come out in
the following release. The
leadbay_new_leadsprompt is gated with them, since every step of thatworkflow calls one of the three.
mcpFirstDeliveryAllTools/listAllPromptskeep both visible to the WORKFLOWS + routing audits, so the flag never reads as
"not registered". Pinned by
mcp-first-delivery-gate.test.tsandprompt-release-gate.test.ts.The backend release itself is not a promote: the last tag
v3.21.6(07-29) is26 commits behind
main, and that gap includesdd4c9a10("the verdict gates the till", the spend fix these tools rely on) alongside
billing/VAT, quota, auth and email changes.
Verification
pnpm -r build,pnpm -r test(1353 passed),pnpm -r typecheck— green locally and in CI on
b866a429.Not verified live. The
/1.6/mcp/*endpoints are staging-only and theavailable tokens authenticate against production only, so the spend gate is
proven through the compiled bundle and unit tests, not a real backend job.
Milan's earlier staging E2E (free search 4.6s / 5 delivered / $0; paid chain
delivering purchased emails; FR parity after the queue drain) predates these
fixes.
No
/evalrun: the changed behaviour (a withheld paid submit) has no existingeval contract, and the harness needs an
.env.evalstaging-token decision.Known backend issues (not fixable here)
lastname.emptyterminal-fails a wholequalify job (reproduced twice).
example_lead.namepollutes seed matching.bulk_qualify_leadsdeprecation is a documented direction only(
docs/mcp-first-lead-delivery.md) — no removal in this diff.Note on test hygiene
One existing test gains a single
confirm: true(
packages/core/test/unit/composite/qualify-leads.test.ts): it purchases anemail channel with no consent, so the new gate correctly withholds it — it was
asserting the behaviour the gate removes. Flagged because the repo rule is
otherwise "never modify existing test files"; all other new coverage is in two
NEW files (
qualify-leads-spend-gate.test.ts,lead-job-status-cursor.test.ts).