@steipete @vincentkoc PR3 for #64230 is live and intentionally narrow: provider-owned OpenAI/Codex tool compatibility only, with no generic-runner dialect, auth, permission, or replay scope mixed in.

Greptile Summary

This PR adds an "openai" family to buildProviderToolCompatFamilyHooks in src/plugin-sdk/provider-tools.ts and wires the resulting hooks into both the OpenAI and Codex providers in extensions/openai/index.ts. It correctly normalizes empty-parameter tool schemas and emits strict-schema diagnostics for remaining incompatibilities, gated to native OpenAI/Codex response routes only.

• P1 — normalizeOpenAIStrictCompatSchemaRecursive corrupts properties: {} maps: The generic recursive descent passes every object value through itself, including the properties map. An empty {} map triggers the "missing params" guard at line 237 and is expanded into a full schema object, changing the API payload for any tool defined with an explicit parameters: { type: \"object\", properties: {} }. The fix is to handle the properties key specially (iterate property schemas by name, not the container), matching what stripUnsupportedSchemaKeywords already does.

Confidence Score: 4/5

Safe to merge after fixing the recursive normalization bug that corrupts explicit empty properties maps.

One P1 correctness bug: the recursive normalizer incorrectly expands any empty {} it encounters, which corrupts the properties: {} container in tool schemas before they are sent to the API. The common parameters: {} and parameters: { type: "object" } inputs work correctly and are covered by tests, but the explicit properties: {} case is untested and broken. All other logic — route gating, violation reporting, Codex/OpenAI routing, hook wiring — looks correct.

src/plugin-sdk/provider-tools.ts — the normalizeOpenAIStrictCompatSchemaRecursive function needs special-case handling for the properties key, matching the pattern used in stripUnsupportedSchemaKeywords.

This is a comment left during a code review.
Path: src/plugin-sdk/provider-tools.ts
Line: 231-244

Comment:
**Empty-object guard corrupts the properties map**

The generic recursive descent calls `normalizeOpenAIStrictCompatSchemaRecursive(value)` for every entry in the record. When the entry is `properties: {}`, the value `{}` is an empty object and triggers the guard at line 237, turning the property-name map into a full JSON Schema `{ type: "object", ... }`. A tool with `parameters: { type: "object", properties: {} }` would therefore have its `properties` field corrupted before the API call.

The fix is to handle the `properties` key specially — iterate its sub-keys and normalize each property schema individually, not the container map itself. This is exactly how `stripUnsupportedSchemaKeywords` treats the same key. Adding a test for the `{ type: "object", properties: {} }` input would catch this regression.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "agents: add openai provider-owned tool c..." | Re-trigger Greptile}

Pushed a minimal follow-up on d733bee4 to align the curated provider-family inventory with the new OpenAI tool-compat hook.

Scope stays narrow to #64230:

• adds toolCompatFamilies: ["openai"] to the curated openai shared-family expectation
• no new compat behavior, no alias bridge, no generic runner changes

I also re-derived the actual shared-family assignment inventory from source before pushing so this stays a contract-alignment fix rather than a speculative patch. This should be ready for maintainer review once GitHub checks refresh on the new head.

Addressed the current execution-correctness review pass on this branch.

This push:

• treats missing parameters as an empty strict object schema for OpenAI/Codex parameter-free tools
• preserves root-object strictness checks when the root schema is an array or primitive
• updates the provider-family contract expectation for the new OpenAI tool-compat family
• preserves paused liveness state when compaction ends in an aborted state
• propagates final replayInvalid / liveness metadata from the runner outcome back into lifecycle state

Local validation:

• pnpm build passed

I also retried focused vitest locally, but the linked-worktree run still fails before loading tests with the existing test/vitest/test/non-isolated-runner.ts module-resolution issue. Because that fails before test execution, I’m using CI as the source of truth for the actual branch logic here.

Rebased this branch onto current main and folded the execution-correctness fixes into the updated upstream lifecycle shape.

What changed in the latest push:

• resolved the merge-conflict on the rebased branch without widening scope
• kept the strict OpenAI/Codex parameter-free schema handling and root-object enforcement
• preserved paused state on aborted compaction
• propagated terminal replayInvalid / liveness metadata into lifecycle end/error emission
• set terminal lifecycle metadata before the prompt-error early returns that immediately return blocked errors

Local validation after the rebase:

• pnpm build

Focused linked-worktree vitest is still unreliable here because of the existing test/vitest/test/non-isolated-runner.ts resolution issue before test execution, so CI remains the source of truth for branch-level test results.

Addressed the rebase fallout on this branch and pushed follow-up commit c0acf1b08e.

What changed:

• restored type-safe handling for the legacy browser SSRF allowPrivateNetwork alias so the rebased branch compiles cleanly against current main
• added the new setTerminalLifecycleMeta noop to the spawn-workspace subscription mocks
• updated the lifecycle test to exercise the intended working -> blocked terminal-error path instead of assuming blocked-by-default

Local validation on the new head:

• pnpm build
• CI=1 pnpm exec vitest run src/agents/pi-embedded-subscribe.handlers.lifecycle.test.ts src/agents/pi-embedded-subscribe.handlers.compaction.test.ts src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test.ts

GitHub is rerunning CI on the new head now.

Landed via rebase merge. Thanks @100yenadmin.

Source tip before merge: f2bb4ca
Landed tip: 5c0d1c6

Local validation before merge:

• pnpm test src/plugin-sdk/provider-tools.test.ts src/agents/pi-embedded-subscribe.handlers.lifecycle.test.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts src/agents/pi-embedded-runner/run.overflow-compaction.test.ts src/agents/pi-embedded-runner/run.incomplete-turn.test.ts src/agents/pi-embedded-runner/run.timeout-triggered-compaction.test.ts extensions/browser/src/browser/config.test.ts
• pnpm check

Merged without waiting for fresh CI per maintainer request.

Follow-up landed on main after the PR branch became non-modifiable from this checkout.

• Follow-up commit: 07edaff
• Verified: pnpm test src/agents/pi-embedded-subscribe.handlers.lifecycle.test.ts src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts src/agents/pi-embedded-runner/run.incomplete-turn.test.ts src/agents/pi-embedded-runner/run.overflow-compaction.test.ts
• Earlier full local test passed with OPENCLAW_TEST_PROJECTS_PARALLEL=2 pnpm test before the final main rebase. Current origin/main has unrelated tsgo/plugin SDK declaration failures outside this PR's final diff.

Thanks @100yenadmin.