[codex] Expose agent harness selection decisions#70760
[codex] Expose agent harness selection decisions#70760steipete merged 6 commits intoopenclaw:mainfrom
Conversation
|
@steipete GPT 5.4 fixes |
Greptile SummaryThis PR refactors Confidence Score: 5/5Safe to merge — no behavioral changes, all remaining findings are P2 style/cleanup suggestions. The change is purely additive observability: a new exported decision type, a pure selection helper, and a debug-gated log call. Runtime paths are equivalent to before, existing tests still pass, and two new tests cover the auto-selection and pinned-PI paths. The only issues are a redundant intermediate No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/harness/selection.ts
Line: 138-142
Comment:
**Redundant intermediate `.map()` on `candidates`**
`candidates` already has the shape `{ harness: AgentHarness, support: AgentHarnessSupport }`. The `.map()` on lines 139–142 re-builds an identical object, adding an allocation with no transformation. The `.filter()` can operate on `candidates` directly.
```suggestion
const supported = candidates
.filter(
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/harness/selection.ts
Line: 125
Comment:
**Misleading `selectedReason` value name**
`"forced_plugin_missing_pi_fallback"` reads as "the PI fallback is missing", but what actually happened is the opposite: the requested plugin harness is missing, so we are *using* the PI fallback. A name like `"forced_plugin_missing_fallback_pi"` or `"forced_plugin_fallback_to_pi"` would match the runtime semantics visible in the log record.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat: expose agent harness selection dec..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR improves observability of embedded agent harness selection by factoring the selection logic into an exported decision helper and emitting structured debug logs that explain why a harness was chosen (and what candidates were considered), without changing the existing AgentHarness SPI or runtime selection behavior.
Changes:
- Add
resolveAgentHarnessSelection(...)to return a selection decision object (selected harness + policy + reason + candidate metadata), and updateselectAgentHarness(...)to delegate to it. - Add debug-gated structured logging in
runAgentHarnessAttemptWithFallback(...)to record selection details. - Update plugin docs to describe the new observability path for diagnosing PI vs Codex harness routing.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/agents/harness/selection.ts | Exposes selection decision helper and emits structured debug logs during embedded runs. |
| src/agents/harness/selection.test.ts | Adds tests for decision observability and ensures no duplicate supports(...) probes. |
| docs/plugins/sdk-agent-harness.md | Documents the structured selection log record for debugging routing decisions. |
| docs/plugins/codex-harness.md | Adds guidance on using the selection log record to spot model-prefix mismatches. |
| export type AgentHarnessSelectionDecision = { | ||
| harness: AgentHarness; | ||
| policy: AgentHarnessPolicy; | ||
| selectedHarnessId: string; | ||
| selectedHarnessLabel: string; | ||
| selectedReason: | ||
| | "pinned" | ||
| | "forced_pi" | ||
| | "forced_plugin" | ||
| | "forced_plugin_fallback_to_pi" | ||
| | "auto_plugin" | ||
| | "auto_pi_fallback"; | ||
| candidates: AgentHarnessSelectionCandidate[]; |
There was a problem hiding this comment.
resolveAgentHarnessSelection(...) is described in the PR as exposing a “pure” decision object, but AgentHarnessSelectionDecision includes a live harness: AgentHarness reference (functions, potentially non-serializable). This makes it awkward to safely persist/emit/JSON-serialize the decision (and increases the risk of accidental logging of function bodies or large closures). Consider returning the selected harness separately (e.g., { harness, decision }) or making the exported decision type omit harness and instead expose only selectedHarnessId/label plus candidates/policy.
02d6f74 to
fe39f12
Compare
|
Landed via squash onto main.
Thanks @100yenadmin! |
Summary
resolveAgentHarnessSelection(...)decision object for the existing embedded harness selector.AgentHarnessSPI and runtime behavior unchanged;selectAgentHarness(...)now delegates to the decision helper.runAgentHarnessAttemptWithFallback(...)so operators can see selected harness id, policy runtime/fallback values, selection reason, and candidate support results.codex/*vsopenai-codex/*routing without redesigning Pi/Codex harness selection.Scope Guard
Validation
node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/harness/selection.test.tspnpm tsgo:core:testpnpm exec oxlint src/agents/harness/selection.ts src/agents/harness/selection.test.tspnpm exec oxfmt --check src/agents/harness/selection.ts src/agents/harness/selection.test.ts docs/plugins/sdk-agent-harness.md docs/plugins/codex-harness.mdnode scripts/run-vitest.mjs run --config test/vitest/vitest.full-core-support-boundary.config.ts test/scripts/lint-suppressions.test.tsgit diff --checkNotes
eb82cc910f.agents/harnesslogger is debug-enabled.supports(...)method is still called exactly once per candidate.