[abhisheksharma001]

Summary

Fixes #8612. The Ghidra bridge handshake can report analysis seeds, disabled-analyzer outcomes and extension results, and checkSessionObservations rejects each report the committed profile did not request. That check ran only inside startSession, which a session called for health or when an eager-start term held. A plain x86_64 ELF whose profile commits none of those terms started the bridge lazily inside client.callTool, so the report was never checked and the first query returned its result with Evidence claiming a profile that did not describe the analysis.

Problem and expected behavior

Trigger: a fake GhidraProviderClientFactory whose start returns a valid handshake plus one uncommitted disabled_analyzers report. With health first, the session fails closed (start 1, call 0, close 1). With list_procedures first, base returns ok: true with start 0, call 1, close 0. Expected: each report binds in both directions, and a session that receives a report its profile did not commit fails closed before it returns any analysis result, whatever the first operation is.

Change and scope

• src/ghidra/GhidraProviderClient.ts: split startSession into checkedStart (start the bridge, bind the handshake) and a thin startSession that also records providerStarted. On the lazy path, when the client reports isReady() === false, run checkedStart before the first query. GhidraProviderClientFactory gains optional isReady, which the production GhidraClient already implements.
• Why narrower than the issue's proposed "run startSession before every query": that proposal changes recovery guidance (a provider lost during its first query would get the open_binary restart guidance) and the MCP startup-cancellation lane's admission point, which the issue flags as needing a maintainer scope decision. This change leaves providerStarted unset on the lazy path, so a provider that dies during its first query keeps the startup guidance pinned by ghidraOomRetention ("Then retry.") and "keeps installation guidance when Ghidra dies before it ever answered". Fakes without isReady keep the lazy call, so the four startup-free fixtures and the startCount 0 pin are unchanged.
• The real client shares one startup attempt, so the bridge still launches once, and callTool's internal start() returns the completed attempt. Cancellation during that start flows through the same shared attempt as today.
• Not included: the MIPS soft-float and RISC-V calling-convention terms stay in the eager condition; the fork branch mentioned in the issue is not merged here.

Contribution scope: lifecycle-adjacent fix within the existing provider adapter; opened as a draft so maintainers can decide whether the narrower gate or the issue's broader proposal is wanted.

Contract and boundary impact

• Semantic owner and earliest changed stage: the Ghidra provider client's startup admission (src/ghidra/GhidraProviderClient.ts); the bridge, launcher and GhidraClient are unchanged.

• Contribution scope fit and linked maintainer decision: the issue requests a maintainer decision for the broad fix; this draft is the narrow alternative for that decision.

• CLI and MCP/tool-catalog contract: none.

• Provider, bridge, target-format, or platform compatibility: every lazily started target (non-PE, no existing project) now binds its handshake before the first query; PE, existing projects and the other eager terms are unchanged.

• Evidence, artifact, provenance, or reconstruction contract: Evidence from a lazily started session can no longer carry a profile that the handshake contradicted.

• Target selection, authority, upstream behavior, and cleanup details: a failed check closes the client before any query, as the health path does.

• Generated metadata, package, or installation impact: none.

• Read existing files or captured data.

Evidence and regression coverage

• Tests added or updated: src/ghidra/GhidraProvider.behavior.test.ts gains "fails closed on an uncommitted analyzer report before a lazily started query" (start 1, call 0, close 1 with list_procedures first) and "binds a lazily started handshake once and queries the ready client directly" (first query start 1 call 1; once the fake reports ready, the second query is call 2 with no further start). analyzerSession accepts an optional isReady.
• Base reproduction: with the base GhidraProviderClient.ts swapped in, both new tests fail (expected true to be false; starts: 0) and the other 194 pass; with this change all 196 pass.
• User-visible CLI/MCP output: the existing ProviderAdapterError for an uncommitted report, now also for a first list_procedures/analyze_function/… query.
• Remaining proof gaps: no real Ghidra run; the uncommitted-report fault itself was not reproduced against a real bridge (the issue reports the same).

Validation performed

• npm run test:local -- src/ghidra/GhidraProvider.behavior.test.ts — 196 passed
• npm run typecheck — passed
• npx oxlint / npx oxfmt --check on the two files — only pre-existing warnings (max-params on projectSessionError, max-depth in the test file)
• npm run test:focused -- tests/boundary/mcp/ghidraStartupCancellationMcp.test.ts tests/boundary/providers/ghidra/ghidraOomRetention.test.ts — 11 passed across the two files, so the pinned startup guidance and both MCP startup-cancellation lanes are unchanged

Compatibility, safety, and release

• Breaking changes or migration steps: none.
• Real Hopper/Ghidra, browser, or OS coverage: not run; simulated launcher and fake-client coverage only.
• Package or release metadata impact: none.
• Security, privacy, process, or containment review: no new process authority; the check closes the owned client through the existing cleanup path.

Author readiness

Human owner: @abhisheksharma001

• I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
• I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
• I declare this PR ready for initial maintainer review.

Before merge

• This PR addresses a concrete problem or an agreed enhancement.
• Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
• The PR has one focused outcome and the title follows type(scope): outcome.
• Related issue is linked, or the reason for not linking one is stated above.
• Tests cover changed observable behavior and meaningful failure paths.
• Owning docs, contracts, and generated metadata are updated where needed.
• User-visible CLI/MCP changes include representative output.
• I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The reviewed changes appear safe to merge.

Summary

The PR checks a lazily started Ghidra handshake against the committed profile before sending the first query.

• Ghidra checks a lazy startup handshake before the first query.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Valid query] --> B{Eager startup required?}
  B -->|Yes| C[Start and check handshake]
  B -->|No| D{Client reports not ready?}
  D -->|Yes| C
  D -->|No| E[Call query]
  C --> F{Reports match profile?}
  F -->|No| G[Close client and return error]
  F -->|Yes| E
  E --> H[Return checked query result]

Loading

_{Reviews (1) · Last reviewed commit: "fix(ghidra): bind the handshake before a..." · Reviewed by Greptile}

[abhisheksharma001]

Closing: #8630 (merged as aba56df) fixed #8612 upstream with the broader checked start on every query while keeping recovery guidance through handshakeEstablishesProvider. This narrower alternative is superseded.