feat(mcp)!: the job ticket is Leadbay's own id; delete the file we kept (product#4005)

[milstan]

Closes leadbay/product#4005. Closes leadbay/product#4039.

Replaces #196, which was closed. Same goal, clean history — that branch was replayed onto a base rather than rebased and silently reverted two merged fixes while deleting their tests. This one is rebased on current main and verified against that specific failure.

The problem, in plain terms

Three tools — qualify a batch of leads, import leads, import-and-qualify — fail on the Leadbay server that Claude and ChatGPT connect to. Nine customers hit it, 53 times. leadbay_import_and_qualify has never once worked there.

The cause: the MCP keeps its own file listing the jobs it has started, so it can hand the assistant a ticket to check on them later. That file is only ever created by the version people install on their own computer. The server version runs a different entry point and never creates it, so every job that needs a ticket fails instantly.

Why we delete the file instead of fixing it

The file exists because of a note in its own header: "while the Leadbay backend doesn't yet issue a real bulk_id." It does now, in two places:

	Covers	What it gives
GET /1.6/mcp/jobs/{id}	search and qualify	server-generated id, kept 30 days, returns 404 to another organization
GET /1.6/notifications	enrichment, import, qualification	per-account, progress counts, a finished flag

Every job the MCP starts already receives one of these and already hands it to the assistant. Our file was a second, worse copy: an id we invented, no protection between customers, and it dies whenever the server restarts.

So the ticket becomes the Leadbay id. Nothing is stored. That also removes the whole question of where to store it — no disk to attach, no volume to configure, and none of the customer's imported rows sitting on our server.

Proven live, twice

Before writing any code, against staging: starting a job returns an id, that id is listable one second later with progress counts, and it resolves from three separate processes with fresh logins.

After, through the MCP server itself: start a job, then check on it from two brand-new server processes. Resolved both times. That is a server restart, live, through the real tools.

What changes for the assistant

Tool	Before	After
enrich_titles, bulk_qualify_leads	our invented id	Leadbay's notification_id + the lead ids
import_leads, import_and_qualify	our invented id	Leadbay's importIds
bulk_enrich_status, qualify_status	demanded our id	take notification_id
import_status	—	takes importIds, which it already accepted

Breaking: bulk_id, handle_id and qualify_id are gone as inputs, with no transition period. They were valid for minutes on a local install and never worked on the server, so there is nothing to transition.

What is kept

A small in-memory guard, alive for five minutes, so that if the assistant fires the same request twice we do not charge the customer twice. It does not need to survive a restart: losing it costs at most one duplicate inside a five-minute window, which is the right trade for not owning a datastore.

This also closes product#4039. The earlier version of that guard had a real flaw, raised in review on #196: while a job was still being started, a second identical request was told "it's running" and handed a ticket that did not exist yet. On the server that is an unrecoverable answer — importIds is the only ticket, so an empty one means the rows are uploaded somewhere the assistant can never find. Fixed by making "still starting" an explicit state the callers have to read, rather than something they infer.

Guarding the failure that caused #4005 in the first place

The original bug was the server being built without the file while every test passed. packages/mcp/test/http-no-store-e2e.test.ts now drives the real server on a real socket and checks that a second, separate request can resolve the id. There is no longer an option to leave out, so the two versions cannot drift apart again.

Checked against how #196 went wrong

• Neither reverted file is reverted here: the hosted-notifications fix and the cancellation wrapper are both intact.
• All seven test files feat(mcp)!: the job handle is the backend's id; delete the bulk store (product#4005) #196 deleted are present.
• pnpm -r build, pnpm -r typecheck, pnpm -r test — 2,177 tests passing.

Also

Eight tool descriptions, four prompts and three shared snippets rewritten. qualify_status had been telling the assistant its ticket was "persisted to ~/.leadbay/bulks.json with a 30-day TTL and survives MCP restart" — every clause of that was false.

Merging this cuts no release

packages/mcp/package.json stays at 0.34.0. auto-tag.yml fires on a push to
main whose diff touches that one file, and from there tags, publishes to npm and
the MCP Registry, cuts a GitHub release and builds the image Argo deploys to
mcp.leadbay.app. release.yml and build-image.yml have no push-to-main trigger
of their own, so leaving that file alone means merging this runs CI and nothing
else. Several PRs land before the next release and the batch gets tested as a
whole first.

Both CHANGELOGs keep their ## 0.35.0 sections — release.yml builds its notes
by matching ## <version> in packages/mcp/CHANGELOG.md, so the entry is
already where the release PR will look for it. That separate PR sets
package.json + server.json and re-titles the sections if the batch ships as
something other than 0.35.0. This is still a breaking change and should read as
one when it does ship.

Review fixes

Four findings, all of them this PR's own regressions.

• long-running-tools swapped qualify_id for notification_id on both
  tools at once, but async import_and_qualify returns import_ids and no
  notification_id at all. Each tool now names its own handle and its own
  status tool.
• enrich_titles told the assistant to poll with {notification_id, lead_ids}. lead_ids is what switches bulk_enrich_status to the per-lead
  path, and on that path titles/email/phone are what make "done" mean the
  roles and channel THIS run asked for. Without them every contact counts, so a
  lead whose CFO was email-enriched months ago satisfies a fresh phone-only CEO
  run and all_done is true on the first poll — enrichment reported as finished
  before anything landed. All three instruction strings carry the scope now.
• import_status was told it "does not re-reconcile records or return
  refreshed leads". It does, and its own outputSchema says so. The
  still_settling, the committing phase and the qualify-the-imported-leads
  step were deleted with that sentence; all three are back.
• A background import that times out no longer parks the rows for ever.
  commitMappings is the MCP's own POST, so a chunk abandoned before it sits
  inert while import_status classifies the row as complete — the user is
  told an import finished that committed nothing. The blocking path already
  hands such an upload to resumeParkedUpload; the wait_for_completion:false
  path only logged, and that path is the one the hosted server now runs for the
  first time. Beyond the literal finding, the resume covers every un-committed
  chunk rather than only the one that threw: chunks are 100 rows, so a larger
  import leaves the later ones parked identically. A dry run is never resumed —
  committing its mappings would turn a validation pass into a real import.

Sweeping the rest of the rewritten prose for the same class turned up more of it.
import_and_qualify has no qualification notification_id — its qualify
phase is a per-lead /web_fetch fan-out, so the backend mints no job; its
notification_ids[] are the FILE-IMPORT ones, and handing one of those to
qualify_status is rejected as the wrong kind. Four surfaces promised that
field anyway. They now split the two launchers: bulk_qualify_leads resumes by
notification_id, import_and_qualify by lead_ids + lens_id, the import
half by import_ids. qualify_status's enrichment redirect had dropped the
poll scope too, so it carries titles/email/phone now.

Review then caught the parked-import fix itself: keying the recovery off
ImportPhaseTimeout("preprocess") missed a backend pre_processing.error and a
transport blip mid-poll, both of which leave the chunk equally uncommitted. The
error type cannot answer that question — the same blip one phase later lands
after the commit, where re-sending it would re-trigger processing. So
completeUploadedChunk signals the commit as it goes out and the loop reads
that flag instead of guessing.

Three new test files, and two existing ones changed — flagged against the
standing rule: import-leads.test.ts and import-and-qualify.test.ts each
script a whole wait_for_completion:false background flow but never wait for
it, so the detached half outlived the test and its calls landed in the next
test's mock. Both now wait. No assertion changed.

pnpm -r build, -r typecheck, -r test green — 2,253 tests.

Not in this change

product#4032 and product#4033 land next, on top of this.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[github-actions]

Synced with main; @leadbay/mcp is 0.34.0 (main shipped 0.33.2).

[milstan]

[Claude]: all five defects fixed. build-and-test green, 2,191 tests.

1 + 2 — per-lead progress and channel-aware done, restored. You were right that neither needed the store. They needed lead_ids, titles, email, phone — all returned by enrich_titles and carried by the agent. bulk_enrich_status takes them as params now and computes the rollup exactly as main did: enrichment_progress:{done,total} per lead, counting only contacts whose requested channel has landed, scoped to the titles this run enriched. A phone-only run no longer counts an email-enriched contact as done. The paragraph telling the agent to redo this by hand with get_contacts is deleted — it existed only to paper over the loss.

3 — the archived dead end is closed. bulk_enrich_status now has the same anyOf as qualify_status, so lead_ids alone is a valid call and answers fully. New shared notifications/read-by-id.ts also searches the archived set and pages, so a job the agent archived with our own acknowledge_notification is still findable. And I removed the two false claims — there is no GET /notifications/{id} on the backend, so the hint now says what the lookup actually does instead of promising a 30-day org-scoped retention.

4 — the four dead instructions are gone, plus two you did not list: enrich-titles declared re_used while the code returns reused, and listed launched_tracker_pending (never returned) while omitting launch_in_flight (returned).

5 — ctx.progress ticks restored with the fan-out, titles/email/phone echoed again, and partial_failures now has coverage.

Tests: 10 new across 3 new files — the phone-only case, title scoping, partial_failures, and finding an archived or paged-past job. None of those had a test.

What a user can do after this that they could not before: start an enrichment, qualification or import on mcp.leadbay.app and poll it — all of it threw BULK_TRACKER_UNAVAILABLE there. Come back to a job in a later conversation or the next day. Poll a job they have already archived.

What they can no longer do: pass bulk_id / handle_id / qualify_id. Those handles lived minutes on local installs and never worked on hosted. No other capability is reduced — that was the point of this round.

Verified: runWithRequestSignal 3/3 vs main, fetchTerminalNotifications 2/2, and the only test files absent from main are the three that tested the deleted store. Live hosted verification after deploy, as you asked.

[claude]

Solid rewrite overall — the launch-guard claim/settle split correctly closes the product#4039 race, and readNotificationById's archived+paginated scan fixes the dead-end from the prior review round. Two issues before merge, both stemming from the bulk_id→notification_id rename not being swept everywhere it needed to be:

1. enrich-titles.ts accidentally added the new notification_id/lead_ids/reused output fields to inputSchema instead of outputSchema — the tool's own declared contract no longer documents the job id it mints. Worse, at least one real consumer (packages/components/src/runtime.ts's enrichment() helper) still reads the deleted bulk_id field and will now silently report every enrichment launch as instantly complete with no job ever polled. getting-started.ts has the same stale reference in its onboarding-tour prompt text.
2. bulk-enrich-status.ts's outputSchema.required still claims notification_id is always returned, but the new lead_ids-only path can omit it.

Neither is exercised by this PR's own tests since the boundary is mocked, so a repo-wide grep for bulk_id/handle_id/qualify_id before merge would be worthwhile given this is explicitly a no-transition-period breaking change.

[claude]

notification_id, lead_ids and reused are declared under inputSchema.properties here, not outputSchema. Their descriptions ("Carry it to leadbay_bulk_enrich_status", "True when an identical launch...was reused") are plainly describing return values — and execute() does in fact return all three (lines 88/94/145). But outputSchema (line 260+) previously documented the equivalent fields as bulk_id/re_used/durability and this PR deletes them without adding replacements back — so the schema for the very field this whole PR introduces as the new job handle (notification_id) is now undocumented in the output, while incorrectly implied to be a launch parameter. EnrichTitlesParams (top of file) confirms execute() never reads notification_id/lead_ids/reused from input, so as inputs they're just inert — this looks like the three properties were pasted into the wrong schema block.

Separately: packages/components/src/runtime.ts's enrichment() helper (not touched by this PR) still reads r.bulk_id from the leadbay_enrich_titles result and calls leadbay_bulk_enrich_status({bulk_id}). With bulk_id gone, bulkId is now always null, so every real launch takes the "no job" branch and reports {all_done:true, no_job:true} immediately — silently faking completion for any artifact/widget built on that SDK helper. packages/components/test/domain.test.ts didn't catch it because it mocks the tool call directly with {bulk_id:"b1"} rather than exercising the real (renamed) core output. Worth a repo-wide grep for bulk_id/handle_id/qualify_id before merge — packages/core/src/composite/getting-started.ts:435 also still tells the onboarding-tour agent to poll "with the returned bulk_id".

[claude]

outputSchema.required lists notification_id as always present, but the new lead_ids-only path (no notification_id passed in) returns an object that omits the notification_id key entirely (...(params.notification_id ? { notification_id: ... } : {}) further down). The declared schema no longer matches what a lead_ids-only call actually returns.

[claude]

Follow-up review on commit 6296313 (previous review at 252dfd9 requested changes).

Both prior findings are fixed: notification_id/lead_ids/reused moved from inputSchema to outputSchema in enrich-titles.ts, with the stale bulk_id reference in runtime.ts's enrichment() helper and getting-started.ts's onboarding prompt both updated to the new handle shape; and bulk-enrich-status.ts's outputSchema.required no longer claims notification_id is always present.

The new code in this diff (per-kind ENRICH_JOB_WRONG_KIND/QUALIFY_JOB_WRONG_KIND guards using inferKind, the ENRICH_JOB_NO_COUNTERS path, and the qualify-status not-found fallback when lead_ids+lens_id are supplied) is well covered by the new bulk-enrich-status-kind.test.ts and qualify-status-kind.test.ts unit tests, and I traced the logic against those tests without finding defects. CHANGELOG/README/prompt-template updates are consistent with the code changes.

No new significant issues found. (Note: I was unable to invoke the review-dismissal endpoint directly in this environment — posting this as a COMMENT review instead, since no outstanding issues remain from the prior CHANGES_REQUESTED review.)

[github-actions]

main shipped 0.33.3. Bump this PR's version above 0.33.3 (package.json + server.json) and re-anchor your CHANGELOG section above main's, then push. The sync bot won't edit CHANGELOG prose.

[github-actions]

main shipped 0.33.4. Bump this PR's version above 0.33.4 (package.json + server.json) and re-anchor your CHANGELOG section above main's, then push. The sync bot won't edit CHANGELOG prose.

[github-actions]

main shipped 0.34.0. Bump this PR's version above 0.34.0 (package.json + server.json) and re-anchor your CHANGELOG section above main's, then push. The sync bot won't edit CHANGELOG prose.

[claude]

Follow-up review on the diff since commit 6296313 (previous review at that commit was COMMENTED, no outstanding issues). The two new fix commits (e81a403 handle-rename correctness, 7170d8a parked-import resume) both check out except for one gap in the parked-upload resume logic — see inline comment. Everything else in the delta (the notification_id/titles/email/phone poll-scope additions in enrich-titles.ts and the long-running-tools/import-status template restorations) matches its commit description and looks correct.

[claude]

This resume guard only fires for ImportPhaseTimeout("preprocess"), but pollPreprocess (called with no try/catch of its own inside completeUploadedChunk, before commitMappings) can also throw IMPORT_PREPROCESS_FAILED when the backend reports pre_processing.error, or a plain transport error mid-poll — neither is an ImportPhaseTimeout. In those cases the current chunk is genuinely uncommitted (the loop never reached commitMappings), but parkedFrom becomes inFlight + 1, which skips it. That chunk is then never handed to resumeParkedUpload and sits parked forever — the exact failure this commit sets out to close, just via a different preprocess-time error than a timeout. The new test (import-leads-parked-resume.test.ts) only exercises the timeout case, so this gap isn't caught.

[claude]

This diff only touches qualify-status error text, notification_id documentation (templates + generated file), and adds a new test — all correct and consistent with the schemas they describe. However, it doesn't touch import-leads.ts, so the outstanding issue from the previous review (parked-upload resume guard only covers ImportPhaseTimeout("preprocess"), missing IMPORT_PREPROCESS_FAILED/transport errors mid-preprocess-poll) is still unaddressed. Keeping changes-requested until that's fixed or explicitly deferred.

[claude]

Follow-up review on the diff since commit 2c33e6f (previous review at that commit requested changes over the parked-upload resume gap).

The outstanding issue is fixed correctly: completeUploadedChunk now takes an onCommitSent callback that fires the instant commitMappings resolves, and runImportInBackground tracks a committed flag per chunk instead of inferring commit state from the error type (ImportPhaseTimeout("preprocess")). This closes the gap the prior review flagged — a backend pre_processing.error or a transport blip mid-preprocess-poll now correctly parks the current chunk too, not just a budget timeout. The three new/updated tests in import-leads-parked-resume.test.ts exercise exactly the three relevant branches (timeout resume, non-timeout preprocess-failure resume, and no-resume-after-commit-sent) and I traced each mock sequence against the code — all check out.

Note: the two await new Promise(...3_000) additions to existing test files (import-and-qualify.test.ts, import-leads.test.ts) technically modify existing test files, against repo convention — but this is transparently disclosed in the commit message, fixes a real cross-test leak (a detached background task's calls landing in the next test's mock), and changes no assertions. Not blocking.

No new significant issues found. (Unable to invoke the review-dismissal endpoint in this environment — posting as COMMENT since no outstanding issues remain from the prior CHANGES_REQUESTED review.)