Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84be5bf6e2
ℹ️ 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".
| const requestToolApproval = host?.approvals?.requestToolApproval; | ||
| const approveToolCall: RunAgentLoopOptions['approveToolCall'] | undefined = requestToolApproval ? | ||
| ((call: ToolCall, tool: ToolDefinition) => requestToolApproval({ call, tool }) ?? Promise.resolve({ approved: false, reason: 'Missing approval port.' })) |
There was a problem hiding this comment.
Preserve approval handler receiver when normalizing host
The new approval bridge calls a detached requestToolApproval function, which drops the original receiver object. If a host provides approvals.requestToolApproval as a class/object method that reads this (for policy state, config, metrics, etc.), this call path can throw or silently misbehave and reject/skip approvals. The previous implementation invoked host.approvals.requestToolApproval(...) as a member call, so this is a regression in the approval flow for host integrations that rely on method binding.
Useful? React with 👍 / 👎.
Summary of candidate implementation
Old paths removed
New owners
Forbidden leftovers checked
src/core/chat/session-turn-preflight.ts:84: return persistPreparedChatSessionTurn({
src/core/chat/session-turn-preflight.ts:105:export function persistPreflightCompactionRunningSeed(args: {
src/core/chat/session-turn-preflight.ts:128:export function persistPreparedChatSessionTurn(args: {
src/tests/unit/core/chat-turn-runtime.test.ts:21: const root = mkdtempSync(join(tmpdir(), 'heddle-turn-session-'));
src/tests/unit/core/chat-turn-runtime.test.ts:203: persistPreflightCompactionRunningSeed({
src/tests/unit/core/chat-turn-runtime.test.ts:228: const preparedSession = persistPreparedChatSessionTurn({
Behavior tests added/updated
Verification run
�[1m�[46m RUN �[49m�[22m �[36mv4.1.2 �[39m�[90m/Users/roackb2/Studio/projects/ProjectHeddle/heddle�[39m
�[32m✓�[39m src/tests/integration/chat/session-submit.test.ts �[2m(�[22m�[2m1 test�[22m�[2m)�[22m�[32m 1�[2mms�[22m�[39m
�[32m✓�[39m src/tests/unit/core/chat-turn-persistence.test.ts �[2m(�[22m�[2m2 tests�[22m�[2m)�[22m�[32m 5�[2mms�[22m�[39m
�[32m✓�[39m src/tests/unit/core/chat-turn-runtime.test.ts �[2m(�[22m�[2m8 tests�[22m�[2m)�[22m�[32m 11�[2mms�[22m�[39m
�[32m✓�[39m src/tests/integration/chat/chat-runtime.test.ts �[2m(�[22m�[2m18 tests�[22m�[2m)�[22m�[32m 27�[2mms�[22m�[39m
�[2m Test Files �[22m �[1m�[32m4 passed�[39m�[22m�[90m (4)�[39m
�[2m Tests �[22m �[1m�[32m29 passed�[39m�[22m�[90m (29)�[39m
�[2m Start at �[22m 12:52:26
�[2m Duration �[22m 406ms�[2m (transform 417ms, setup 0ms, import 752ms, tests 44ms, environment 0ms)�[22m
/Users/roackb2/Studio/projects/ProjectHeddle/heddle/src/core/chat/README.md
0:0 warning File ignored because no matching configuration was supplied
✖ 1 problem (0 errors, 1 warning)
Known risks
Remaining review questions
This is a candidate implementation for review, not a milestone-complete claim.