[plugin sdk] Derive tool target paths for hooks#75605
Conversation
|
Codex review: needs real behavior proof before merge. Summary Reproducibility: not applicable. this is a feature PR rather than a bug report. Current-main source inspection confirms the requested hook metadata is absent, so verification is PR diff review, SDK contract review, CI, and real behavior proof. Real behavior proof Next step before merge Security Review detailsBest possible solution: Land the additive SDK metadata only after maintainer API review, redacted real setup proof, exact-head checks green or explained, and the remaining review threads addressed or resolved. Do we have a high-confidence way to reproduce the issue? Not applicable: this is a feature PR rather than a bug report. Current-main source inspection confirms the requested hook metadata is absent, so verification is PR diff review, SDK contract review, CI, and real behavior proof. Is this the best way to solve the issue? Yes, subject to approval and proof: a host-owned parser registry with advisory Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 150b869cf8c9. |
891ef40 to
11e2a1d
Compare
ac3f694 to
a86c942
Compare
|
Fixed in |
a86c942 to
5861ad6
Compare
![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: a86c942080
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Adds a host-owned mechanism to derive file target paths (initially for apply_patch) and surfaces those derived facts to plugin tool-policy hooks, so plugins can make path-based decisions without re-parsing tool envelopes.
Changes:
- Introduce
extractApplyPatchTargetPaths()and a small host tool-param parser registry (deriveToolParams) that derivesderivedPaths. - Plumb
derivedPathsthrough trusted tool policies andbefore_tool_callhook events (including recomputation after trusted policy param rewrites). - Add unit + e2e/contract coverage and update docs/baselines/changelog for the new hook field.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/plugins/trusted-tool-policy.ts | Rebuild per-policy tool events with host-derived metadata (supports recomputation after param rewrites). |
| src/plugins/host-tool-param-parsers.ts | Adds the host-owned tool param derivation registry (currently apply_patch). |
| src/plugins/host-tool-param-parsers.test.ts | Tests for derivation behavior, empty cases, and malformed params. |
| src/plugins/hook-types.ts | Extends PluginHookBeforeToolCallEvent with optional derivedPaths. |
| src/plugins/contracts/host-hooks.contract.test.ts | Contract coverage for clearing stale derivedPaths across trusted policy rewrites. |
| src/agents/pi-tools.before-tool-call.ts | Computes/stamps derivedPaths onto trusted-policy and before_tool_call events, and recomputes post-rewrite. |
| src/agents/pi-tools.before-tool-call.e2e.test.ts | E2E coverage for derivedPaths propagation and recomputation after trusted policy rewrites. |
| src/agents/apply-patch-paths.ts | Implements lightweight path extraction for apply_patch envelopes. |
| src/agents/apply-patch-paths.test.ts | Unit tests for the extractor across markers, move targets, dedupe, and line endings. |
| docs/plugins/hooks.md | Documents event.derivedPaths for before_tool_call. |
| docs/.generated/plugin-sdk-api-baseline.sha256 | Updates generated SDK API baseline hashes. |
| CHANGELOG.md | Notes new SDK surface: host-derived tool target paths for hooks. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 535bcb648e
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57749fe090
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4928f6140f
ℹ️ 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".
d0df425 to
444daac
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
444daac to
bbb77a9
Compare
bbb77a9 to
a6b9699
Compare
openclaw-barnacle
Bot
added
the
triage: needs-real-behavior-proof
|
Rebased onto upstream/main 69c1487 (2026-05-08) Resolved 3 conflicts:
Force-pushed to |
Upstream changed the default acp.stream.deliveryMode from `live` to `final_only`. The 4 ACP-streaming tests in dispatch-from-config.test.ts relied on the old default to exercise the block-streaming path, so they now never call sendBlockReply. Add explicit `deliveryMode: "live"` to each affected test config to keep them on the live path. Mirrors upstream commit ea51160.
|
Thanks @100yenadmin. I could not push the rebased maintainer fixups back to the fork (403), so I landed this via maintainer replacement #79705. It keeps the derived Landed in e4bae42. |
Summary
apply_patchpath extractor that derives target paths from add/update/delete/move markers.derivedPathsonto trusted-policy and before_tool_call events.This replaces the consolidated #73384 / #74483 stack with a smaller fresh-main slice. The previous consolidated branch became extremely brittle under constant upstream churn: rebases repeatedly invalidated generated baselines, review threads, and CI assumptions, which made the PR harder to review than the underlying changes warranted. This PR files one hook/seam area at a time so maintainers can review, test, and merge each contract independently without re-opening the whole workflow stack.
Links
Non-goals
apply_patch.Test Plan
pnpm test src/agents/apply-patch-paths.test.ts src/plugins/host-tool-param-parsers.test.ts src/agents/pi-tools.before-tool-call.e2e.test.tspnpm check:test-typespnpm lint:coregit diff --checknpm run check:no-conflict-markers