fix(mcp): re-land leadbay_getting_started with the 8 review findings fixed - #177
Draft
ArtyETH06 wants to merge 3 commits into
Draft
fix(mcp): re-land leadbay_getting_started with the 8 review findings fixed#177ArtyETH06 wants to merge 3 commits into
ArtyETH06 wants to merge 3 commits into
Conversation
…people-getting-started" This reverts commit 2220fe8.
…rough P1 — the paid reveal was told to pass a singular `leadId`, a key leadbay_enrich_titles does not read; it would have been dropped and the confirmed one-contact reveal would fall back to the wishlist default, spending on several leads. Both surfaces now say `leadIds: [<id>]`. P1 — the tool returned the module-level manifest BY REFERENCE, and the server mutates results in place to attach `_meta`. Deep-clone per call. P1 — the live eval recorded every tool call as `ok:true, output_len:0` and never read the tool_result, so a call that errored still satisfied `required_calls` and reached the judge as a success. Results are now parsed into the evidence, and required/order read only successful calls. P1 — the no-spend scenario relied on a missing fixture, but the live runner ignores fixtures: the regression it guards would have been paid for in real credits. The paid launch is now blocked in the spawned server, before the network. P2 — gate 4 needs write tools the walkthrough does not get on a read-only deployment; it now declares `requires_tools` and degrades to the close instead of offering a button that dies on the click. P2 — `allowed_calls` was declared by scenarios and enforced nowhere. P2 — the unscoped STOP line fought ENDING B's required closing offer. P2 — WORKFLOWS.md still required the CRM and scheduling gates that were cut before the feature shipped. Also renumbers to 0.29.0: 0.28.0 is already published to npm from a different commit, so the CHANGELOG section claimed a shipped version. Co-Authored-By: Claude <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
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.
Re-lands the walkthrough backed out by #176, with every finding from the automated review on #175 fixed.
Merge #176 first — this branch carries the revert, so after #176 lands its diff is just the feature, corrected.
The 8 findings
leadId— a keyleadbay_enrich_titlesnever reads. It would be dropped, and with noleadIdsthe tool falls back to the top of the wishlist: the confirmed one-contact reveal could spend on several leads.leadIds: ["<id>"], and say why the array matters.executereturned the module-level manifest by reference, and the server mutates results in place to attach_meta.update_available/_meta.notifications— so per-call metadata stuck to the singleton and could replay on a later call.structuredCloneper call.ok:true, output_len:0and never read thetool_result. A call failingBAD_INPUT/LAST_PROMPT_REQUIREDstill satisfiedrequired_callsand reached the judge as a success.required_calls/required_orderread only calls that succeeded.forbidden_callsstill reads every call that fired — attempting a banned call is the violation.no-unprompted-enrich-spendleaned on a missing fixture to make a paid launch fail — but this runner ignores fixtures and hits the real API. The regression it guards would have been paid for in real credits.noSpend: trueblocks/leads/selection/enrichment/launchinside the spawned server, before the network. The freemode:"discover"half still runs live.leadbay_enrich_titles+leadbay_bulk_enrich_status, which a read-only deployment (LEADBAY_MCP_WRITE=0) filters out — the tour offered a button that dies on the click.requires_tools+unavailable; the agent checks its own tool set first and closes at gate 3 instead.allowed_callswas declared by scenarios and read by nobody, so a scope eval could fan out into real tools and still be decided by a judge score.WORKFLOWS.mdstill required a CRM check at gate 4 and a gate 5 scheduling handoff — gates that were cut before the feature shipped, making the normative criteria impossible to satisfy.Version
0.28.0is already published to npm from a different commit, so the CHANGELOG section claimed a shipped version. Renumbered to 0.29.0 acrosspackage.json,server.json, the plugin manifest and its npx pins.Tests
pnpm -r test→ 1351 passing (up 9), typecheck + build clean. New coverage in two new files:getting-started-codex-review-fixes.test.tsandeval-runner-evidence-and-spend-guard.test.ts.Two pre-existing eval helpers are modified (
live-session-runner.ts,live-mcp-server.ts) — the two P1 eval defects have no other correct home, and #175 already modifiedllm-judge-shared.tson the same grounds. No existing*.test.tsspec is touched.Not covered
/evalrun in this PR. The runner's own evidence handling changed, so the honest move is to re-run the two gate scenarios once revert(mcp): back out leadbay_getting_started (#175) pending Codex fixes #176 lands and paste that block — the previous run's numbers no longer describe this code.leadbay_research_lead_by_id, and claiming a phone/email it never received) is still unfixed — out of scope here, which was the 8 review findings plus the version.Closes https://github.com/leadbay/product/issues/3952