Repository navigation
fix(verification): compare retained Ghidra function evidence - #8541
Open
ShuaixinHuang wants to merge 1 commit into
Open
ShuaixinHuang wants to merge 1 commit into
ShuaixinHuang wants to merge 1 commit into
Conversation
18 of 22 tasks
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
[Low impact] The reviewed code appears safe to merge; no actionable defect was found. SummaryThe verifier now uses one shared helper to compare function records already kept by its MCP session.
Reviews (1) · Last reviewed commit: "fix(verification): compare retained Ghid..." · Reviewed by Greptile |
This branch has not been deployed
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.
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, reportingmax_input_elements: 1024. The same record compared through retained references returnsunchangedand 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-functionunchanged, unresolved-callunknown, profile checks, source Evidence links, full bundle-record equality, and session cleanup assertions remain. Fullanalyze_functionreplies 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
scripts/lib/native-value-e2e.mjs; both verifier comparison controls use the same producer.compare_functionsretained-Evidence input, no new command/tool/schema.evidence_id; full source and comparison records remain in the bundle.Expected effects of the existing real-provider verifier remain:
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
d48e472fa6a664cacbb400560e673533005795cecontains 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.unchangedorunknown, retains the exact source link and full source/comparison records, and refuses a missing retained ID.resource_constraint; patched source is restored byte-for-byte infinally. This is a controlled reversion after mechanical extraction, not a claim that the newly exported helper existed on main.For evidence-bearing changes:
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.Compatibility, safety, and release
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.Author readiness
Human owner: @ShuaixinHuang
Before merge
type(scope): outcome.This replacement preserves the prior PR discussion and validation history at #8258. It reuses the verified published head and requires its own hosted CI.