Repository navigation
Conversation
|
@greptileai please review this MIPS I contribution at head 7eeb8ed. It adds a narrow recordless ELF32 ET_EXEC lane with explicit o32 headers, pins Ghidra's language, and preserves floating-point uncertainty. The real-provider verifier passed for little- and big-endian fixtures through production MCP, including loaded bytes, saved Evidence, replay and unchanged input hashes. The little-endian fixture also passed the production CLI. The fixtures were never executed. This does not claim a firmware corpus or Windows verification. |
 [Low impact] The reviewed changes appear safe to merge, with no actionable issue found. SummaryAdds MIPS admission controls for both byte orders and checks cleanup using each provider’s actual run UUID.
Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Capture prelaunch identities] --> B[Check refusal controls through CLI and MCP]
B --> C[Analyze admitted controls with Ghidra]
C --> D[Read validated provider run UUID]
D --> E[Retain native ownership evidence]
E --> F[Close target and transport]
F --> G[Sweep outer UUID and every provider UUID]
G --> H{Analysis and cleanup passed?}
H -->|Yes| I[Remove workspace and report passed]
H -->|No| J[Fail and retain workspace]
Reviews (12) · Last reviewed commit: "chore: merge upstream into MIPS verifier..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@greptileai please re-review head 652ee67. Both verifier findings are addressed. The fixture check now observes the conditional Child stderr is inherited so startup and cleanup diagnostics remain visible when verification fails. A fresh real Ghidra 12.1.4/macOS arm64 run passed both endian MCP fixtures, byte/branch checks, retained FP uncertainty, snapshot replay, and LE production CLI. Original target hashes remain unchanged; targets were never executed. Native Linux/Windows, firmware corpus, FP semantics and the delay-slot relationship remain unverified. Pre-push typecheck/lint passed. Full upstream CI limitations are linked in the description. |
Codex Review Summary: ✅ CompletedThis 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. |
![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: 652ee67cf0
ℹ️ 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".
652ee67 to
992216e
Compare
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 992216e8d1
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
992216e to
2b5c34b
Compare
2b5c34b to
d8630ea
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8630eaf13
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Preserve the MIPS I refusal until load-delay semantics are verified. Decode typed GNU attribute presence before making recordless ABI assumptions, and exercise both refusal paths plus existing MIPS32 controls through CLI/MCP. BREAKING CHANGE: ELF32 MIPS inputs with SHT_GNU_ATTRIBUTES are refused until those ABI declarations are parsed and reconciled. Preserve the original input and use a tool that understands its declared ABI; stripping metadata does not establish compatible semantics.
Bobtoshi
changed the title
|
@greptileai please review narrowed head 5ec1a57. Both high-priority findings are addressed structurally. GNU attributes are detected by their typed section header and refused before ABI assumptions or section-payload allocation. The metadata regression fails on the previous reader. The MIPS I admission/profile and its provider claims were removed: generic Ghidra's immediate load assignment does not establish MIPS I load-delay semantics, so decoding an integer probe is insufficient. MIPS I stays unsupported with an explicit reason. 133 tests plus static/docs/discovery checks passed. Both endian MIPS I and GNU-attribute controls were refused through production CLI/MCP; GNU readelf independently decoded the soft-float attribute. Existing recordless MIPS32 controls still produced real Ghidra dossiers and exact loaded bytes. Input hashes remained unchanged and no target code was executed. GNU attribute parsing/reconciliation and MIPS I processor semantics remain future work. The description includes the refusal's compatibility change and migration guidance. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ec1a57a29
ℹ️ 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".
|
@greptileai please review latest head 05e2366. The replacement verifier now isolates both CLI/MCP children from ambient REA settings, propagates a process-run token, requires verified empty descendants before success, and pins GNU readelf to the C locale. A real both-endian run passed under a poisoned ambient seed/analyzer configuration and conflicting parent locale, with unchanged input hashes and no target execution. All 135 focused tests and static/docs checks passed. MIPS I admission remains withdrawn; GNU ABI attributes are refused until parsed and reconciled. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05e23666d8
ℹ️ 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".
morluto
added
area: domain
Share tagged readiness observations, retain partial provider facts, authenticate bridge identity, bound diagnostics, and preserve cancellation and lost-session reasons. Fail closed for unparsed MIPS ABI attributes and macOS saved Hopper databases without a script-capable bridge route. Reviewed fixes: #2010, #2108, #2513, #2695, #2731, #2771, #2828, #2867, #2868, #2870, #2880, #2919, #2945, #2961, #2981, #2998, #3000, #3002. Validation: 285 focused source/boundary tests passed. Real Ghidra, IDA, Linux Hopper, and Windows provider runs were not repeated. BREAKING CHANGE: MIPS targets with unresolved GNU ABI attributes and saved Hopper databases on macOS are refused rather than advertised as supported.
|
Heads up: main covers this in its own way in d4bd65c. The code is in Edit (24 Oct): I re-read the branch on e389016, without running anything. The follow-up is here. 🤖 Generated with Claude Code (Sonnet 5.5, verified by Opus 5.5) 🦊 |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Bobtoshi
changed the title
|
@greptileai please review latest head e389016. The branch is merged with current upstream, preserving its GNU attribute parser and incomplete-declaration gate. Both endian partial-section-header regressions, 149 focused tests and static/docs/discovery checks pass. Real CLI/MCP refusal and MIPS32 positive controls pass on Ghidra 12.1.4/macOS arm64 with unchanged source hashes and verified empty descendants. Windows is now rejected before workspace/provider startup. No target execution or native Linux/Windows claim. Maintainers: the upstream Actions runs show action_required for fork-origin code. Please approve the current-head PR workflows so required CI can run; local results are reported above. |
|
Happy to help unblock this — the branch had fallen into merge conflicts against
Verification on the merged tree: |
|
TL;DR: I re-read the branch on e389016 without running anything. The overlap I raised on 14 Oct is gone, because the branch now uses main's GNU attribute parser. It still adds a real-Ghidra verifier and a MIPS I test that main lacks. But its MIPS I refusal now contradicts main's #7014 (97194c4), and the branch conflicts with main. Edit (7 Nov): I ran this PR at head 11edd85 in a VM, and the results are here. This follows my heads-up of 14 Oct, which asked whether the PR still adds something main lacks.
Conflict with main. GitHub says the branch conflicts. A text merge of main (aba487e) and the head conflicts in 2 of the 5 files that both sides changed. One is GitHub's checks (secondhand): On e389016 the The author's reply says 149 focused tests and a real Ghidra 12.1.4 run on macOS arm64 passed. I did not check that. Not run
Thanks, Bobtoshi, for merging main, so the branch carries main's parser. Thanks, IvenKooLab, for the notes on the conflicts. What I read
Test setup🤖 Written with Claude Code (Sonnet 5.5, verified by Opus 5.5), posted with my OK 🦊 |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Bobtoshi
changed the title
|
@greptileai please review head 2d2f2f8. Current main is merged. The obsolete MIPS I refusal and duplicate declaration tests are removed; this now adds only source-owned real-Ghidra boundary controls and independent run-token cleanup. The live orphan probe and 134 focused tests cover the previous PPID-only false green. Real Ghidra 12.1.4/macOS arm64 controls pass. The body distinguishes executed checks, retained host diagnostics and the full typed-lint/package/Linux/Windows checks that were not completed. Human readiness declarations remain for the owner to complete. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
Codex technical review completed at @IvenKooLab, thanks for the earlier merge and test notes. Could you, or another qualified MIPS/process-ownership contributor or maintainer, review this bounded change and explicitly accept human ownership under CONTRIBUTING? This is a request for a human handoff. The readiness declarations remain unchecked, and @Bobtoshi remains the listed owner until another human explicitly accepts. No human review has been attested by the agent. |
|
@greptileai please review |
|
The exact-head fork CI run for @morluto or another maintainer: please approve the current-head fork-origin upstream CI run and the title workflow, then review the PR for merging. They currently report |
|
TL;DR: I ran this PR at head 11edd85 in a VM on Linux x86-64, with real Ghidra 12.1.4. The new MIPS boundary check passes for both byte orders, and the PR's one new test file passes on Node 22, 24 and 26. Still open: GitHub says the branch conflicts with main. I also couldn't tell which files make up the author's 135 tests. Edit (7 Nov): The author named the files behind the 135 tests, and they all pass at 3569da2. Results are here. This follows my review above, which was a read only. I tested the head alone, not merged with main.
Unguarded reads. The verifier reads Still not run
Thanks, Bobtoshi, for removing the MIPS I refusal. Your both-endian controls from macOS arm64 also pass on Linux x86-64. Test setup
CommandsAll in the VM,
I grepped each test file for 🤖 Written and tested with Claude Code (Sonnet 5.5, verified by Opus 5.5), posted with my OK 🦊 |
|
Thanks @CassieFox23 for running the verifier on Linux. The 135 tests for
The branch now includes main @morluto, could you approve the refreshed branch’s upstream workflows when you have a chance? CI, PR title, and CodeQL currently show The fork run has found two failures that also appear on upstream main. The Knip failure matches #9295. The check passes locally when testing #9300’s proposed entry in a temporary config. The Apple verifier failure matches the upstream JSON error at The Apple failure is reproducible in the malformed-file checks. The command owner returns stdout through |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3569da2921
ℹ️ 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".
| detail: "complete", | ||
| }), | ||
| ); | ||
| assert.ok(dossier.pseudocode.length > 0); |
There was a problem hiding this comment.
When the recordless MIPS-I override is missing or wrong, Ghidra still imports this fixture but auto-selects a 64-bit MIPS language (as documented in docs/providers/ghidra-mips.md:46-49), so nonempty pseudocode and exact read_bytes can both pass without exercising the pinned MIPS:<LE|BE>:32:default lane. The sibling real-MIPS verifier explicitly checks inspect_native_load_image().observations.language_id; add that assertion for this MIPS-I control before reporting its admission as exercised.
AGENTS.md reference: AGENTS.md:L31-L33
Useful? React with 👍 / 👎.
|
TL;DR: The three files you named make 135 tests, and all pass at 3569da2 on Node 22, 24 and 26. The real-Ghidra MIPS check passes there too, in both byte orders. I ran them at 3569da2, which has main 0b028cc merged in, in a Fedora 44 VM on Linux x86-64.
That is 135 on each Node, the same as your count. I also ran On this head, only Greptile Review and Validate author declaration have run so far. The CI, PR title and CodeQL runs still wait for approval. Thanks, Bobtoshi, for naming the three files. Commandsnpm ci --offline
npm run test:prepare
npm run test:focused -- src/ghidra/GhidraMipsProfile.test.ts tests/boundary/filesystem/mipsAbiFlags.test.ts tests/boundary/process/verifierTokenCleanup.test.ts
GHIDRA_INSTALL_DIR=<ghidra 12.1.4> JAVA_HOME=<jdk 21.0.12.1> node scripts/verify-real-ghidra-mips-boundaries.mjsInstall and 🤖 Written and tested with Claude Code (Opus 5.5), posted with my OK 🦊 |
Adds real Ghidra MIPS boundary checks and catches surviving provider processes after reparenting. Ghidra gives each target its own run ID, so the verifier records the actual session ID and native ownership evidence while the provider is running. It checks every recorded provider ID and the outer verifier ID after transport closure. Cleanup failure preserves the diagnostic workspace.
The controls cover both byte orders. Declared-o32 MIPS I and unsupported GNU soft float are refused through CLI and MCP. Recordless MIPS I and MIPS32 produce real Ghidra dossiers and exact loaded bytes through MCP. The verifier checks input hashes and does not execute the targets. Failed analysis prevents a pass. Cleanup evidence covers the recorded provider sessions.
Merged main
0b028cc75. Git carried the documentation edits intodocs/providers/ghidra-mips.md. The seven-file diff still contains the verifier, tests, documentation and its Knip entry. Production MIPS admission and metadata parsing follow main.Validation on
3569da292180726033ae454c791619a29a3e4439with Node 24.22.1 and npm 11.16.0src/ghidra/GhidraMipsProfile.test.ts(75),tests/boundary/filesystem/mipsAbiFlags.test.ts(57), andtests/boundary/process/verifierTokenCleanup.test.ts(3). The live orphan regression follows the production launcher’s token override and confirms identity-checked teardown.11edd85fb, both byte orders with real Ghidra and the cleanup tests on Node 22, 24 and 26. The real verifier was not rerun on Linux or Windows after this refresh. Local full-suite and package E2E checks were not run. The controls do not establish MIPS I load-delay correctness.cause.stdoutfield atdispatch.mjs:397. A local probe reproduced the error and passed both malformed-file fixtures after readingVerifierCommandExitError.snapshot.stdout.text, preserving the CLI and MCP refusal assertions. That probe covers the failing path.Related to #2734.
Author readiness
Human owner: @Bobtoshi