Repository navigation
Conversation
Document large-ASAR source_paths inventory, integrity_policy for re-signed unpacked companions, and explicit provider_id when multiple deep providers are configured. Fixes morluto#10501
Codex Review Summary: ❌ FailedThis 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. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
[Low impact] The changes affect agents that use the reverse-engineering skill and its Cursor mirror. The documentation appears safe to merge, with a non-blocking correction needed for the inventory-first workflow. Findings
Fix with agent prompt### Issue 1
.agents/skills/reverse-engineer-anything/references/javascript-applications.md:32-35
The new route calls `inspect_artifact` before analysis, but this guidance prescribes `integrity_policy: "record-and-continue"` only for `analyze_javascript_application` and `extract_artifact`. With re-signed `.unpacked` companions, inspection uses the default `fail` policy and returns `artifact_integrity_mismatch` before the agent reaches analysis. Include `inspect_artifact` in the policy guidance and update the Cursor mirror too.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryUpdates the authored skill and Cursor mirror with:
One non-blocking documentation fix remains: apply the integrity policy to Supersession could not be checked because GitHub access was unavailable. No security issue was identified in the reviewed changes. Reviews (1) · Last reviewed commit: "docs(skill): clarify ASAR inventory-firs..." · Reviewed by Greptile |
| `detail: "summary"`. Pass `integrity_policy: "record-and-continue"` on | ||
| `analyze_javascript_application` and `extract_artifact` when an Electron `.app` | ||
| bundle's `app.asar.unpacked` native companions were re-signed after ASAR | ||
| creation and retaining declared-vs-observed digest contradictions is intended; |
There was a problem hiding this comment.
Inventory still rejects re-signed companions
The new route calls inspect_artifact before analysis, but this guidance prescribes integrity_policy: "record-and-continue" only for analyze_javascript_application and extract_artifact. With re-signed .unpacked companions, inspection uses the default fail policy and returns artifact_integrity_mismatch before the agent reaches analysis. Include inspect_artifact in the policy guidance and update the Cursor mirror too.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .agents/skills/reverse-engineer-anything/references/javascript-applications.md
Line: 32-35
Comment:
**Inventory still rejects re-signed companions**
The new route calls `inspect_artifact` before analysis, but this guidance prescribes `integrity_policy: "record-and-continue"` only for `analyze_javascript_application` and `extract_artifact`. With re-signed `.unpacked` companions, inspection uses the default `fail` policy and returns `artifact_integrity_mismatch` before the agent reaches analysis. Include `inspect_artifact` in the policy guidance and update the Cursor mirror too.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Summary
Fixes #10501.
Clarify skill routing for large Electron
app.asartargets: inventory first withopen_binary+inspect_artifact(detail: "summary"), pass narrowedsource_pathstoanalyze_javascript_application, useintegrity_policy: "record-and-continue"when.unpackedcompanions were re-signed after packing, and select an explicitprovider_id(or callbinary_session({})) when multiple deep providers may be configured.Problem and expected behavior
Skill v62 routed ASAR/Electron trees straight to
analyze_javascript_applicationwithout size/scope guards, omittedintegrity_policyfor post-packing re-signed.unpackedbinaries, and documentedprovider_idforida/kunabut not multi-providerghidra/hopperambiguity. Agents then hit whole-ASAR host deadlines,artifact_integrity_mismatch, andcapability_unavailable(selection_reason: "ambiguous"). Expected: progressive-disclosure routing guards inSKILL.mdwith details inreferences/javascript-applications.md.Change and scope
Authored skill only (
.agents/skills/reverse-engineer-anything/plus regenerated.cursor-plugin/skills/mirror):SKILL.mdASAR bullet — inventory-first +source_pathsfor large packages;integrity_policy: "record-and-continue"when retaining contradictions.SKILL.mdnativeopen_binarybullet — explicitprovider_idorbinary_session({})when multiple deep providers may be present.references/javascript-applications.md— large-ASAR inventory pattern next to the existingdetail: "summary"note; Electron.unpackedintegrity policy guidance.No runtime, catalog, or provider behavior changes.
Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.
Contract and boundary impact
open_binaryguidance.integrity_policyand provider selection behavior).docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs andskills/; edit authored instructions in.agents/skills/reverse-engineer-anything/and never commit build outputs): Cursor plugin skill mirror regenerated; no package version change.Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.
Evidence and regression coverage
integrity_policy/ multi-provider behavior already covered by existing MCP boundary tests (asarUnpackedExtractionMcp, Electron integrity paths)..appre-run in this PR; wording follows contracts already enforced in tests and guides.Validation performed
node scripts/generate-cursor-plugin-skill.mjs --check— passednpm run check:fast— typecheck and lint passedCompatibility, safety, and release
Author readiness
Human owner: @BeqaKs
Before merge
type(scope): outcome.