fix(mcp): the 8 Codex findings on leadbay_getting_started (#175) - #179
Merged
Conversation
…ough Codex reviewed #175 six minutes AFTER it merged, so nothing gated the merge. Fixed forward rather than reverted: nothing shipped (auto-tag.yml only fires on packages/mcp/package.json, which #175 never touched), so main carrying the feature reaches no user, and a revert + re-land would be two merges and a history that adds, removes and re-adds 3,900 lines. P1 — the paid reveal could charge for the whole batch. Gate 4 told the agent to call leadbay_enrich_titles with a singular `leadId`. That key does not exist on the tool, which reads only `leadIds?: string[]`, so it was dropped and the confirmed call fell back to the account's DEFAULT WISHLIST SELECTION with confirm+email set — revealing and charging far beyond the one lead the user agreed to. Now `leadIds: [id]`, always the array, with the reason attached in both the prompt and the manifest so a later trim can't restore the shorter form. P1 — the manifest was returned by reference. The server attaches _meta.update_available / _meta.notifications to results IN PLACE, so those stuck to the module-level singleton and could resurface on a later call in the same process, including notifications already acknowledged. Returns a structuredClone now. P1 — eval evidence was not trustworthy. runSessionLive stamped every call `ok: true, output_len: 0` and never revisited it from the tool_result, so a call failing with LAST_PROMPT_REQUIRED or BAD_INPUT still satisfied required_calls and reached the judge as a success. Tool results are now matched back by tool_use_id and the runner only counts calls that actually succeeded. This means the earlier 5/5/5/5 was softer evidence than reported. P1 — the overdeliver scenario could spend real money. It relied on the absence of a launch fixture to block the paid reveal, but the live runner ignores fixtures and calls the real API. Replaced with `no_paid_calls`, asserted against real tool INPUTS (confirm/email/phone/enrich/titles) before the judge ever runs. P2 — the tour dead-ended under LEADBAY_MCP_WRITE=0: it stays exposed via compositeReadTools but gate 4's leadbay_enrich_titles is write-gated and absent there. Gate 4 now carries a branch that ends the tour after gate 3 instead of offering a button whose tool cannot run. P2 — WORKFLOWS.md still required a CRM-connector check and a gate 5 scheduling handoff. Both gates were removed, so the normative contract was unsatisfiable by the shipped tour. P2 — the STOP include forces a verbatim final line, which contradicted Ending B's "the offer is the last thing you see". Scoped: the offer is the last PROSE; STOP is a machine marker, and must never displace it. P2 — allowed_calls was declared by scenarios and never checked, so a consent scenario could fan out to extra real tools and still be judged on prose alone. Adds getting-started-codex-fixes.test.ts (manifest identity, leadIds array, read-only branch) and eval-runner-guards.test.ts (the three harness guards). 1,349 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
Live eval, post-fix run: the overdeliver scenario scored IA 3 against a floor
of 4, on a single criterion —
"The offer runs two sentences and lists three specific things Zoe covers
('tuning your lens to your market, wiring the CRM push into your own setup,
and getting the daily run automated end to end'), which exceeds 'about one
sentence' and edges toward promotional copy."
The judge was right, and the fault was mine rather than the agent's: the prompt
demanded "one sentence and the link" and then supplied a two-sentence sample
listing three things. The agent copied the sample, as it should. A rule whose
own example breaks it is not a rule.
Both surfaces now carry a one-sentence example, with the length restated as
binding and the reason attached — enumerating everything Zoe covers turns an
offer into promotional copy, which is exactly what someone who just said "I'm
done" doesn't want.
Also drops a stale "six steps" from the completes-all-gates criteria, left over
from the cut to four gates.
NF moved 4 -> 5 on that same run: the fabricated "working phone number and
email" from the pre-fix run is gone.
Co-Authored-By: Claude <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: 43c22afab9
ℹ️ 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 — the structural spend guard was dead on arrival. The scenario declared
`no_paid_calls` at its TOP LEVEL; the runner read `sc.mission.no_paid_calls`.
So the guard I added to replace the fixture-based one never executed once, and
the no-confirmation walkthrough was again unprotected against a regression
calling leadbay_enrich_titles with confirm/email/titles against the real API.
A guard that exists in review but not at runtime is worse than none, because it
reads as covered. Moved under `mission` (canonical, where every other contract
field lives) and the runner now accepts either placement, so a misplaced flag
can never silently disarm it again.
P2 — the error-envelope regex marked healthy calls failed. It scanned the whole
JSON for any uppercase `code`, but a nested one is routine in a SUCCESSFUL
result: leadbay_account_status returns user+org alongside
`quota_error: { code }` when the quota subrequest fails, which the prompt is
written to handle. Now parses and inspects the TOP LEVEL only (error / isError
/ ok:false), falling back to the host's is_error flag for non-JSON.
P2 — per-turn expect_calls only needed a call to fire, not to succeed. The
judge's own pre-check builds from raw evidence and ignores output_summary, so a
BAD_INPUT / LAST_PROMPT_REQUIRED result satisfied a turn mechanically. Same
success filtering as required_calls now applies per turn.
P2 — allowed_calls as a strict whitelist flagged correct behaviour. A prompt's
own mandated setup calls are not stray: leadbay_daily_check_in opens with
leadbay_account_status, so pull-leads-offers-enrich-top-leads would report a
correct precheck as a scope breach. The whitelist now includes the prompt's
declared expected_calls and every turn's expect_calls.
P2 — gate 4's read-only check sat AFTER the instruction to fire the widget, so
an agent following the prompt in order still offered a button it could not
fulfil before reaching the "don't fire it" line. Moved ahead of the widget.
Five new assertions in eval-runner-guards.test.ts, including one that pins the
guard to where scenarios actually declare it — the failure mode that made the
P1 invisible. 1,353 tests pass.
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: 7696915a91
ℹ️ 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".
…on of mine P1 — I edited an existing test file. getting-started.test.ts is pre-existing on this branch and the repo rule is new coverage goes in new files only; I changed an assertion there rather than move it. Restored untouched, and the manifest wording now satisfies the assertion that was already there while the new length checks live in getting-started-codex-fixes.test.ts where they belong. P2 — the gate-4 args placeholder showed `leadIds` as a scalar. `args` is the LITERAL shape to pass and the tool declares an array, so following it verbatim earns a BAD_INPUT at the reveal — the opposite of what the leadIds fix was for. The values are placeholder descriptions rather than real values, so the shape travels in the placeholder: `[<id>] — an ARRAY, always`. P2 — per-turn success searched the whole session, so a success on any other turn masked a failure on the turn that declared the expectation. Scoped to `c.turn === i + 1` on both the success and failure lookups. P2 — the whitelist folded in the prompt's entire expected_calls, which is its full success path, not its setup. offers-enrich-top-leads scopes itself to pull_leads plus a free preview; admitting daily_check_in's whole workflow would have voided that. Narrowed to one named setup call (leadbay_account_status) with the reasoning attached. P2 — the memory exemption matched "leadbay_get_agent_memory", which is not what any memory tool is called, so a run following the memory protocol was reported as a stray. Matches the agentMemoryTools catalog now instead of a guessed prefix. P2 — the confirmed-reveal criterion landed on WORKFLOWS #51, the NO-confirmation row, where a paid reveal must never happen. Made conditional so the normative contract can't be read as expecting a call it forbids. Three new guard assertions. 1,357 tests pass. 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: 0aee96d1f2
ℹ️ 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 — the guard could only ever report a spend that had already happened. It
inspected tool inputs AFTER runSessionLive returned, by which point the live
server had called leadbay_enrich_titles({titles, confirm:true, email:true})
against the real API and charged the account. An overdeliver eval must not be
able to spend the quota it exists to prove is safe.
Moved to the HTTP boundary in live-mcp-server.ts: when the runner arms
LEADBAY_EVAL_NO_PAID_CALLS (before the session, not after), the client's
request methods refuse any path matching the enrichment launch endpoint. That
holds regardless of which tool, argument shape, or future code path reaches for
it — the input assertion stays as a second net for shapes an endpoint match
can't see, but it is no longer the thing standing between a regression and a
real charge.
P2 — required_order walked the raw call sequence. A failed account_status, then
a successful pull_leads, then a successful account_status satisfied
account_status -> pull_leads, though the real order was reversed. It now walks
the SUCCEEDED sequence, and the failure message reports that sequence.
P2 — a tool call whose result never arrived stayed recorded as a zero-length
success, so a session truncated by max-turns or an early SDK exit could satisfy
required_calls on calls nobody ever saw resolve. Provisional records now start
not-ok and are only promoted by a real tool_result, and anything still pending
at session end is swept with the reason named. Unproven is not the same as fine.
Three new guard assertions, including one asserting the kill switch is armed
BEFORE runSessionLive rather than after. 1,360 tests pass.
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: 0b7e2111b4
ℹ️ 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".
The kill switch blocked only /leads/selection/enrichment/launch. That left a
real hole on the exact scenario it guards: at gate 3 a regression calling
leadbay_prepare_outreach({enrich:true}) delegates to leadbay_enrich_contacts,
whose paid requests go to
/leads/{leadId}/enrich/contacts/{contactId}/enrich
/leads/{leadId}/contacts/{contactId}/enrich (org fallback)
Neither matched, so both would have reached the real API and charged the
account before the post-hoc assertion could fail — the same class of miss the
switch was introduced to close, one level down.
Now matches on the ENRICH segment across all three routes, so an endpoint
rename doesn't silently reopen it. Free discovery paths (job_titles, preview,
status) are deliberately excluded: the tour depends on them and they cost
nothing.
Tested as BEHAVIOUR, not source text. eval-no-spend-paths.test.ts extracts the
live patterns and runs the real request paths through them — three paid routes
must block, five free ones must not. A grep could not tell a guard that covers
every reveal route from one that covers only the bulk endpoint, which is
exactly how this leaked; the corresponding grep in eval-runner-guards.test.ts
now defers to it.
Also restores deny(), which my own PAID_PATHS rewrite had deleted while leaving
its call site — the switch would have thrown ReferenceError on first use.
1,368 tests pass.
Co-Authored-By: Claude <noreply@anthropic.com>
ArtyETH06
added a commit
that referenced
this pull request
Aug 11, 2026
Only WORKFLOWS.md conflicted, and additively on both sides: main's getting-started walkthrough (#175/#179) claimed rows 50/51 while this branch already had 50/51/52 for the delivery tools. Both sets kept, ours renumbered to 52/53/54 — same convention as the earlier collision on this branch, where main's rows keep their numbers. The contract blocks follow the same order. main's row 51 references #50 as its own overdeliver twin, which still resolves correctly. 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
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.
Codex reviewed #175 at 13:49 UTC — six minutes after it merged at 13:43, so its findings never gated the merge. Fixed forward on top of
main.Why not a revert. Nothing shipped:
auto-tag.ymlonly fires onpackages/mcp/package.json, which #175 never touched, so there was no tag, no release, no publish — npmlatestis still0.28.0from717b02c. A revert + re-land would be two merges and a history that adds, removes and re-adds 3,900 lines. This PR is ~500 lines of actual fixes. (#176 and #178 closed; #177 superseded.)P1 — the paid reveal could charge for the whole batch
Gate 4 told the agent to call
leadbay_enrich_titleswith a singularleadId. That key doesn't exist on the tool — it reads onlyleadIds?: string[]— so it was dropped and the confirmed call fell back to the account's default wishlist selection withconfirm/emailset. It would have revealed and charged far beyond the one lead the user agreed to. NowleadIds: [id], always the array, with the reason carried in both prompt and manifest so a later trim can't quietly restore the shorter form.P1 — the manifest was returned by reference
The server attaches
_meta.update_available/_meta.notificationsto results in place, so those stuck to the module-level singleton and could resurface on a later call in the same process — including notifications the user had already acknowledged. Returns astructuredClonenow.P1 — eval evidence was not trustworthy
runSessionLivestamped every callok: true, output_len: 0and never revisited it from thetool_result, so a call failing withLAST_PROMPT_REQUIREDorBAD_INPUTstill satisfiedrequired_callsand reached the judge as a success. Results are now matched back bytool_use_id, and only calls that actually succeeded count.This means the
MM 5 / IA 5 / NF 5 / TSF 5quoted on #175 was softer evidence than it appeared. Re-running now with the guard in place.P1 — the overdeliver scenario could spend real money
It relied on the absence of a
/leads/selection/enrichment/launchfixture to block the paid reveal — but the live runner ignores fixtures and calls the real API. Replaced withno_paid_calls, asserted against real tool inputs (confirm/email/phone/enrich/titles) before the judge ever runs.P2s
compositeReadTools, but gate 4'sleadbay_enrich_titlesis write-gated and absent underLEADBAY_MCP_WRITE=0. Gate 4 now ends the tour after gate 3 rather than offering a button whose tool can't run.WORKFLOWS.mddrift. It still required a CRM-connector check and a gate 5 scheduling handoff; both gates were removed, making the normative contract unsatisfiable.STOPvs the 1:1 offer. The include forces a verbatim final line, contradicting Ending B's "the offer is the last thing you see". Scoped: the offer is the last prose;STOPis a machine marker and must never displace it.allowed_callsnever enforced. Declared by scenarios, collected, and ignored — so a consent scenario could fan out to extra real tools and still be judged on prose alone.Gates
prompts:checkclean · build · typecheck · 1,349 tests (+7). New:getting-started-codex-fixes.test.ts(manifest identity,leadIdsarray, read-only branch) andeval-runner-guards.test.ts(the three harness guards). No existing test file modified.Live eval re-running against the real API with the corrected harness; I'll post the scores when it lands rather than quote the pre-fix ones.