Skip to content

fix(verification): compare retained Ghidra function evidence - #8541

Open
ShuaixinHuang wants to merge 1 commit into
morluto:mainfrom
ShuaixinHuang:codex/fix-native-verifier-evidence-references
Open

ShuaixinHuang wants to merge 1 commit into
morluto:mainfrom
ShuaixinHuang:codex/fix-native-verifier-evidence-references

Conversation

@ShuaixinHuang

@ShuaixinHuang ShuaixinHuang commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Supersedes #8491 at the author's explicit request to resubmit this contribution. This new PR uses the same published branch and HEAD 2bf92eb91964d136256116badbf50001dfabedf0. Original discussions and validation are preserved at #8491. The historical results below retain their original execution points; this new PR requires its own hosted CI and a new human declaration for this PR ID.

Supersedes #8258 at the author's explicit request to resubmit the existing PR. This replacement reuses verified head 2bf92eb and preserves the original discussion and validation record at #8179. Historical validation and its limitations below remain at their stated execution points. This new PR must pass its own hosted CI.

Summary

Fixes #8172. The real-Ghidra verifier sends two inline copies of complete function Evidence to compare_functions, exceeding the default MCP input allowance for sufficiently detailed functions. Use exact references to the records already retained by the same MCP session.

Problem and expected behavior

The comparison should run within the default transport budget and keep its original result and provenance assertions. A local compiled stdio MCP probe with explicitly synthetic, artifact-bound function Evidence containing 1200 assembly rows rejects the inline request with resource_constraint, reporting max_input_elements: 1024. The same record compared through retained references returns unchanged and links the original Evidence.

Change and scope

Both the complete-function and unresolved-call verifier controls now use one request producer, compareRetainedFunctionEvidence, with { kind: "retained-evidence", evidence_id } for each operand. Existing response/error parsing, complete-function unchanged, unresolved-call unknown, profile checks, source Evidence links, full bundle-record equality, and session cleanup assertions remain. Full analyze_function replies are still retrieved and checked inline; only the redundant comparison input changes.

No input-limit increase, environment forwarding, provider behavior, schema, authority, timeout, resource setting, or comparison semantics change. The separate JDK-selection report #8171 is outside this PR.

Contract and boundary impact

  • Semantic owner and earliest changed stage: comparison request construction in scripts/lib/native-value-e2e.mjs; both verifier comparison controls use the same producer.
  • Contribution scope fit and linked maintainer decision: ordinary verification-workflow correction using an existing public input contract; new capability scope decision not applicable.
  • CLI and MCP/tool-catalog contract: existing compare_functions retained-Evidence input, no new command/tool/schema.
  • Provider, bridge, target-format, or platform compatibility: no provider or bridge changes. The real-Ghidra verifier still requires its existing Ghidra/JDK/native compiler prerequisites.
  • Evidence, artifact, provenance, or reconstruction contract: comparison consumes the exact retained evidence_id; full source and comparison records remain in the bundle.
  • Target selection, authority, upstream behavior, and cleanup details: existing real-provider workflow unchanged. Local regressions start and close owned Node/MCP children, import synthetic Evidence and perform no engine analysis or target execution.
  • Generated metadata, package, or installation impact: none.

Expected effects of the existing real-provider verifier remain:

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

The retained-reference change adds no target interaction; existing verifier bridge-script execution and owned temporary-file/project responsibilities remain with their current owners. Executable target bytes are not modified.

Evidence and regression coverage

  • Base d48e472fa6a664cacbb400560e673533005795ce contains both duplicated inline request sites. The original public-MCP input shape reproduces the reported default transport refusal using explicit synthetic Evidence; this does not establish a real Ghidra run on this host.
  • The new regression executes the actual shared verifier request producer through real compiled stdio MCP, in separate complete and partial coverage sessions. Each first confirms that the large inline request is refused at the unchanged 1024-element allowance. The verifier request then succeeds with unchanged or unknown, retains the exact source link and full source/comparison records, and refuses a missing retained ID.
  • Controlled reversion of only the shared request operands to the original inline payload makes both final regression cases fail at resource_constraint; patched source is restored byte-for-byte in finally. This is a controlled reversion after mechanical extraction, not a claim that the newly exported helper existed on main.
  • The first partial fixture omitted required coverage limitations and failed admission. The final controlled red and green runs use the corrected schema-valid fixture; that prerequisite failure is not counted as production regression evidence.

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

  • npm run test:focused -- tests/boundary/process/nativeValueVerifierComparison.test.ts src/domain/functionComparison.test.ts src/config/mcpInputBudget.test.ts tests/boundary/mcp/functionComparisonReferenceFactsMcp.test.ts — 3 prerequisite build tasks passed; 4 complete files, 103 tests passed.
  • npm run check:fast — typecheck and lint passed, 2/2 tasks, 26.46s.
  • npm run verify:test-discovery — all 2884 files discovered exactly once.
  • Normal precommit passed; both committed files match the tested SHA256 hashes. Normal prepush passed, 2/2 cached tasks in 230ms; working tree clean.

Compatibility, safety, and release

  • Breaking changes or migration steps: none; existing public retained-reference contract.
  • Real Hopper/Ghidra, browser, or OS coverage: Windows x64, pinned Node 24.22.1/npm 11.16.0, actual local compiled stdio MCP with explicit synthetic facts. No full verify:ghidra, real Ghidra/JDK, macOS/Linux or full repository suite result is claimed. The report's real-provider gate must still be rerun on an equipped host.
  • Package or release metadata impact: none. Owning docs and public contracts do not change.
  • Security, privacy, process, or containment review: no new authority or external service; tests close owned MCP/process resources and use temporary synthetic bundle files. Existing provider and input-budget safeguards remain active.

Author readiness

Human owner: @ShuaixinHuang

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

This replacement preserves the prior PR discussion and validation history at #8258. It reuses the verified published head and requires its own hosted CI.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

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-01T15:19:22.131592Z 2bf92eb PR opened
ℹ️ 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

greptile-apps Bot commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Low impact] The reviewed code appears safe to merge; no actionable defect was found.

Summary

The verifier now uses one shared helper to compare function records already kept by its MCP session.

  • The native-value verifier compares functions by their retained Evidence ID.

Reviews (1) · Last reviewed commit: "fix(verification): compare retained Ghid..." · Reviewed by Greptile

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant