[kaoru0822-kitauji]

What broke: A Ghidra session whose first query was not health, on a target with no eager-start condition, started the bridge inside callTool, so a handshake report the profile did not commit was never checked and the query returned its result.
The fix: Every query now runs the existing checked start first, so the session fails closed with ProviderAdapterError and closes before any query runs; there is still one launch.
The risk: Low. Recovery guidance is deliberately unchanged, and the first query now waits for the handshake in the provider client instead of inside callTool, through the same shared startup attempt. Not run against real Ghidra.

Summary

Fixes #8612.

checkSessionObservations rejects handshake reports the analysis profile did not commit, but only startSession runs it, and a query reached startSession only when an eager-start condition held. Any other session now gets the same check before its first query.

Problem and expected behavior

Trigger: a plain x86_64 ELF with no committed seeds, disabled analyzers or extensions, whose handshake carries one of those reports, and whose first query is, for example, list_procedures.

Violated invariant: each report binds in both directions, so a session that receives a report its profile did not commit fails closed before it returns any analysis result, whatever the first operation is.

This also reached mutating operations: on the base revision set_address_name was dispatched to such a session.

Change and scope

All production changes are in src/ghidra/GhidraProviderClient.ts:

• execute runs startSession before every query. The real client shares one startup attempt, so callTool reuses that launch. Because the check runs per query, a provider restarted within the session is checked again.
• startSession is now the only client.start call in the provider client. The PE inspect_native_data_type path reuses the session info it already holds instead of starting again unchecked.

Paths and reports considered

Path	Outcome
execute, non-health, no eager term	Fixed. This is the reported path.
execute, raw client.start on the PE data-type path	Removed. It ran after a checked start, but could start again unchecked.
executeIfReady	Never starts the bridge. Returns the retained failure after a rejection.
Provider restarted within a session	Checked again by the same per-query start.
Reopen after a rejection	open_binary retires the rejected client; its replacement launches and is checked.
Rejection whose cleanup fails	The error reports incomplete cleanup; replacement stays refused until cleanup can be retried.

The five reports checkSessionObservations binds (analysis seeds, disabled analyzers, extension results, MIPS soft-float storage, RISC-V calling convention) are all tested. The PDB report is not profile-bound, so it is unchanged.

Decision for maintainers: recovery guidance

The issue proposes the same fix but notes that it would change recovery guidance, and asks for a maintainer scope decision before implementation. No maintainer has replied. This PR avoids that change rather than deciding it:

• The former eager-start condition stays as handshakeEstablishesProvider, and selects only whether a completed handshake counts as a running provider.
• A provider lost during the first query keeps the guidance it has today on each kind of session. The two tests the issue names as pinning the old guidance keep their expectations: "reports a kill during a cgroup OOM kill as a memory constraint" passes unmodified, and "keeps installation guidance when Ghidra dies before it ever answered" changes only its fake start.

Removing that condition would give one rule, but it would also put "Call open_binary" in front of one-shot CLI runs on ELF and Mach-O targets. exitedProviderMessage has no caller-lifetime split, and PE one-shot runs already see this text (both source-derived). That is a separate decision, so I left it out. If you want one rule, it means deleting the condition and updating those two tests, and probably a lifetime split for that message.

Not included

• Existing-project sessions still bind only extension results. The bridge reads these reports from the persisted program, and ghidraProjectHistoryEvidence records them as historical project reports rather than outcomes of this launch. The issue also excludes these sessions from its scope. This is source-derived; I did not add a test for it.
• I added no gate for two narrow gaps: executeIfReady between handshake settlement and the check, and callTool's own internal start, which a close() landing just after the check could relaunch. Both are runs of microtasks with no macrotask in them (source-derived) and both already exist on eager-start sessions, so the complexity is not justified.
• fix(ghidra)!: load ESP-IDF flash code in Ghidra's default address space #8464 adds a riscv eager-start term for its own elf_load_preparation report. This PR removes the need for a new eager-start term, but the two will conflict textually at that condition.

Contribution scope: ordinary fix within existing boundaries; the required maintainer scope decision for new capabilities is not applicable. The reporter's request for a decision is covered above and is why "Material scope" below stays unticked.

Contract and boundary impact

• Semantic owner and earliest changed stage: createGhidraProviderClient owns the handshake check; the changed stage is execute, before dispatch. The raw client.start on the PE data-type path is removed as superseded. handshakeEstablishesProvider is retained for recovery guidance only.
• Contribution scope fit and linked maintainer decision: not applicable (fix); see "Decision for maintainers" above.
• CLI and MCP/tool-catalog contract: no schema or catalog change. A session with an uncommitted report now returns execution_failure from its first query instead of a result. Both adapters reach this through the provider client; MCP is tested here and the CLI path is source-derived.
• Provider, bridge, target-format, or platform compatibility: no bridge change. Conforming sessions behave as before.
• Evidence, artifact, provenance, or reconstruction contract: Evidence can no longer claim a profile that a report contradicts on these sessions. Conforming results are unchanged.
• Target selection, authority, upstream behavior, and cleanup details: a rejected session closes through the existing client.close() path, and failed cleanup stays retained and retryable.
• Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

• Read existing files or captured data.
• Launch or attach to a process.
• Inject instrumentation or run scripts/code.
• Change process memory, registers, files, or configuration.
• Contact network services.
• Recover protected code or work around protections.
• Documentation or usability only; no target interaction.
• Not sure yet; investigation or maintainer guidance needed.

(These are the existing Ghidra session effects, including the bridge scripts; this PR adds none. The one launch is unchanged, and a rejected session now runs no query.)

Evidence and regression coverage

• Tests added or updated:
  • src/ghidra/GhidraProvider.behavior.test.ts: 5 reports x 7 first operations (health, list_documents, list_procedures, list_strings, inspect_native_load_image, procedure_assembly, set_address_name), each pinning starts: 1, calls: 0, closes: 1 and the reason.
  • Same file, reviewer counterexamples: retained rejection with no second launch or query, including executeIfReady; a reopened client; a restart within the session; concurrent callers on one shared startup; failed rejection cleanup retried on close; refused input starts nothing; caller cancellation reaches the checked start.
  • Same file, guidance pins: first-query loss on a session committing startup inputs, and loss after health, both give reopen guidance. These pass on base and with the fix, by design.
  • tests/boundary/mcp/ghidraReopen.test.ts: public MCP workflow with the production client, a real child and a real socket, using the simulated bridge. Covers rejection before the first query, no query reaching the bridge, release of the child and runtime root, retry, reopen, and retained failed cleanup.
  • Updated: tests/boundary/mcp/ghidraStartupCancellationMcp.test.ts observes callers in start, where they now wait; "commits exact provider..." pins startCount 1 (it was 0); three fakes stubbed start to fail and now succeed. tests/fixtures/fakeGhidra.mjs takes optional handshakeReports.
• Base reproduction or other evidence (executed, with only GhidraProviderClient.ts reverted to 8250bf232): 42 failed, 203 passed across GhidraProvider.behavior.test.ts, ghidraReopen.test.ts and ghidraStartupCancellationMcp.test.ts. All five health cases pass; the non-health cases fail because the query ran. The two MCP regressions fail because the query returned a normal result, and the two cancellation cases fail because callers never reach start.
• User-visible CLI/MCP output (executed, MCP list_strings as the first query):

  {"error":{"code":"execution_failure","retryable":false,"details":{"provider_id":"ghidra","operation":"list_strings","diagnostics":{"reason":"The Ghidra session reported analysis seeds that the profile did not commit."}}}}

  (message and remediation are omitted here; they carry the existing generic rea doctor text.)
• Cost (executed, three alternating rounds of 2,000 warm list_strings on the production client against the simulated bridge, so this says nothing about real Ghidra's cost): 1 launch on both; cold first query 60 ms on base and 59-60 ms with the fix; warm 45-48 µs per query on base and 45-46 µs with the fix; test-process CPU 55-67 µs and 56-57 µs per query; response 3,078 bytes on both. Agent tokens and process-family peak memory were not measured; the response size is unchanged.
• Remaining proof gaps: no real Ghidra was run, and none exists on this machine, so no real bridge fault was reproduced and how often one occurs remains NOT_ENOUGH_INFO. The MCP tests skip on Windows and did not run there. There is no CLI-level regression test.

Boundary and lifecycle regressions covered: callers sharing one handshake, cancellation during startup, a rejected session's cleanup and its retry, restart within a session, and reopen. Cancellation during the rejection's own close() is not separately tested; close() takes no signal and this path is shared with eager-start sessions.

For evidence-bearing changes:

• Observed, derived, and inferred claims remain distinguishable.
• Artifact identity, source provenance, and failed attempts remain preserved.
• Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Run on macOS arm64, Node.js 24.22.1, on the commit in this PR.

• npx vitest run src/ghidra tests/boundary/mcp/ghidra tests/boundary/providers/ghidra tests/boundary/providers/startupCloseWaiters tests/boundary/filesystem/ghidra tests/process-global/ghidra src/providers src/application/binary/CompositeProvider — 78 files passed; 1600 tests passed, 3 skipped.
• npm run check:fast — passed.
• npm run format:check — passed.
• npm run docs:check and npm run metadata:check — not run; no docs or catalog changed.

Compatibility, safety, and release

• Breaking changes or migration steps: none for conforming sessions. A session with an uncommitted report now fails, which is the intended fix.
• Real Hopper/Ghidra, browser, or OS coverage: none. Ghidra is simulated at the bridge; macOS only.
• Package or release metadata impact: none.
• Security, privacy, process, or containment review: this narrows behaviour by failing closed before mutating operations too. It adds no process, authority or data flow.

Author readiness

Human owner: @kaoru0822-kitauji

• 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.

Declarations entered by Claude Code on the owner's standing written instruction.

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-02T06:59:03.516052Z	0e463c5	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no blocking issues were found.

Summary

Every Ghidra query now checks the handshake before dispatch. The PE data-type path also reuses the checked session information instead of calling client.start again.

• Ghidra checks each query's handshake before running it.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Query] --> B[Check operation and input]
  B --> C[Start or reuse shared startup]
  C --> D{Handshake reports match profile?}
  D -->|Yes| E[Dispatch query]
  D -->|No| F[Close owned provider]
  F --> G[Return retained failure]
  G --> H[Reopen retires rejected client]

Loading

_{Reviews (2) · Last reviewed commit: "Merge origin/main into fix/8612-ghidra-h..." · Reviewed by Greptile}