Repository navigation
feat(approval-gate): state-as-truth refactor — direct policy consult + reactive turn_state event + reload recovery #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 26 commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
5f2f58c
feat(harness-node): complete reactive turn-step wake
ytallo becd57a
feat(turn-orchestrator): add policy_function_id to config
ytallo be518c0
refactor(turn-orchestrator): keep policy_function_id at top level, ma…
ytallo 77e5696
refactor(turn-orchestrator): thread policy_function_id to dispatchWit…
ytallo e4cd599
refactor(turn-orchestrator): make consultBefore policy_function_id re…
ytallo 26e05a7
refactor(turn-orchestrator): call policy::check_permissions directly …
ytallo b005d6c
refactor(turn-orchestrator): align hook with approval-gate types, mat…
ytallo 41af78e
refactor(approval-gate): wake turn::step via direct trigger, drop dur…
ytallo bff14b0
chore(approval-gate): refresh stale wake-path descriptions
ytallo 595a3ab
refactor(approval-gate): delete gate-subscriber and before-hook topic…
ytallo 311ac70
chore(approval-gate): update worker manifest description
ytallo bf2829a
chore(approval-gate): drop orphan policy_function_id from approval-ga…
ytallo ae77d5f
docs(approval-gate): describe direct policy consult + state-trigger wake
ytallo 43684c7
docs(approval-gate): drop stale event row and deleted-type listing
ytallo e13d1e8
fix(turn-orchestrator): re-emit approval_requested when parking on ap…
ytallo 859550e
feat(turn-orchestrator): emit turn_state_changed on every turn_state …
ytallo 4c05c20
refactor(turn-orchestrator): align on-turn-state-changed error handli…
ytallo 0b9437d
feat(console/web): add pending-approvals diff helper
ytallo 87c004c
feat(console/web): add turn-state mirror reducer
ytallo 06ebe02
feat(console/web): derive pending approvals from turn_state with relo…
ytallo 14f8874
refactor(console/web): tighten turn_state_changed integration per review
ytallo d71166d
refactor: drop approval_requested signal event in favor of turn_state…
ytallo d1d59ce
refactor: drop approval_resolved signal event; denial flows through f…
ytallo 7e701ed
docs(approval-gate): describe state-derived pending-approval signalling
ytallo 3a30d5a
docs(turn-orchestrator): distinguish on-record-written from on-turn-s…
ytallo bc59d1b
chore: drop dead consultPolicy export, consolidate STEP_FN_ID to one …
ytallo 1c315dd
fix(turn-orchestrator): close permissions bypass and reject same-stat…
ytallo e32b74d
refactor(turn-orchestrator): drop legacy approval_required parameter
ytallo 949c4f2
fix: address Codex re-review P2 findings
ytallo 18a1f5f
refactor(turn-orchestrator): remove outdated comments for clarity
ytallo adfd072
fix(turn-orchestrator): restore durable retry on direct turn::step fa…
ytallo a1a0aa0
fix(turn-orchestrator): close pre_approved trigger throw + cap after-…
ytallo 8292ad0
feat(turn-orchestrator): add turn::get_state, remove direct state::ge…
ytallo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
63 changes: 63 additions & 0 deletions
63
console/web/src/lib/backend/pending-approvals-store.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| import { describe, expect, it } from 'vitest' | ||
| import { | ||
| diffPending, | ||
| type PendingApproval, | ||
| } from './pending-approvals-store' | ||
|
|
||
| describe('diffPending', () => { | ||
| it('returns all entries as added when previous list is empty', () => { | ||
| const next: PendingApproval[] = [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::fs::write', args: { path: '/tmp/x' } }, | ||
| ] | ||
| expect(diffPending([], next)).toEqual({ | ||
| added: next, | ||
| removed: [], | ||
| }) | ||
| }) | ||
|
|
||
| it('returns all entries as removed when next list is empty', () => { | ||
| const prev: PendingApproval[] = [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::fs::write', args: { path: '/tmp/x' } }, | ||
| ] | ||
| expect(diffPending(prev, [])).toEqual({ | ||
| added: [], | ||
| removed: prev, | ||
| }) | ||
| }) | ||
|
|
||
| it('detects a single addition', () => { | ||
| const prev: PendingApproval[] = [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::fs::write', args: {} }, | ||
| ] | ||
| const next: PendingApproval[] = [ | ||
| ...prev, | ||
| { function_call_id: 'fc-2', function_id: 'shell::shell', args: {} }, | ||
| ] | ||
| expect(diffPending(prev, next)).toEqual({ | ||
| added: [next[1]], | ||
| removed: [], | ||
| }) | ||
| }) | ||
|
|
||
| it('detects a single removal', () => { | ||
| const prev: PendingApproval[] = [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::fs::write', args: {} }, | ||
| { function_call_id: 'fc-2', function_id: 'shell::shell', args: {} }, | ||
| ] | ||
| const next: PendingApproval[] = [prev[0]!] | ||
| expect(diffPending(prev, next)).toEqual({ | ||
| added: [], | ||
| removed: [prev[1]], | ||
| }) | ||
| }) | ||
|
|
||
| it('is keyed by function_call_id (treats objects with same id as same entry)', () => { | ||
| const prev: PendingApproval[] = [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::shell', args: { command: 'ls' } }, | ||
| ] | ||
| const next: PendingApproval[] = [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::shell', args: { command: 'rm' } }, | ||
| ] | ||
| expect(diffPending(prev, next)).toEqual({ added: [], removed: [] }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| /** | ||
| * Pure helpers for diffing two pending-approval lists by function_call_id. | ||
| * Used by the translate layer to derive synthetic `fcall-start` / | ||
| * `fcall-end` events from successive `turn_state_changed` updates. | ||
| * | ||
| * No state lives here — the per-session list lives in the closure of | ||
| * `realStream` (one chat = one mirror) and is passed in/out as plain | ||
| * arrays. Keeping this file framework-free makes it unit-testable in | ||
| * isolation. | ||
| */ | ||
|
|
||
| export interface PendingApproval { | ||
| function_call_id: string | ||
| function_id: string | ||
| args: unknown | ||
| } | ||
|
|
||
| export interface PendingDiff { | ||
| added: PendingApproval[] | ||
| removed: PendingApproval[] | ||
| } | ||
|
|
||
| export function diffPending( | ||
| prev: PendingApproval[], | ||
| next: PendingApproval[], | ||
| ): PendingDiff { | ||
| const prevIds = new Set(prev.map((e) => e.function_call_id)) | ||
| const nextIds = new Set(next.map((e) => e.function_call_id)) | ||
| return { | ||
| added: next.filter((e) => !prevIds.has(e.function_call_id)), | ||
| removed: prev.filter((e) => !nextIds.has(e.function_call_id)), | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| import { describe, expect, it } from 'vitest' | ||
| import type { AgentEvent } from '@/types/iii-agent-event' | ||
| import { createTurnStateTranslator } from './translate' | ||
|
|
||
| describe('createTurnStateTranslator', () => { | ||
| it('emits fcall-start { pendingApproval: true } when a new entry appears', () => { | ||
| const translate = createTurnStateTranslator() | ||
| const event: AgentEvent = { | ||
| type: 'turn_state_changed', | ||
| event_type: 'state:updated', | ||
| new_value: { | ||
| state: 'function_awaiting_approval', | ||
| awaiting_approval: [ | ||
| { | ||
| function_call_id: 'fc-1', | ||
| function_id: 'shell::fs::write', | ||
| args: { path: '/tmp/x' }, | ||
| }, | ||
| ], | ||
| }, | ||
| old_value: { | ||
| state: 'function_execute', | ||
| awaiting_approval: [], | ||
| }, | ||
| } | ||
| expect(translate(event, 'sess-a')).toEqual([ | ||
| { | ||
| kind: 'fcall-start', | ||
| functionId: 'shell::fs::write', | ||
| input: { path: '/tmp/x' }, | ||
| pendingApproval: true, | ||
| functionCallId: 'fc-1', | ||
| sessionId: 'sess-a', | ||
| }, | ||
| ]) | ||
| }) | ||
|
|
||
| it('emits nothing when the awaiting_approval list is unchanged', () => { | ||
| const translate = createTurnStateTranslator() | ||
| const same = { | ||
| state: 'function_awaiting_approval', | ||
| awaiting_approval: [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::shell', args: {} }, | ||
| ], | ||
| } | ||
| translate( | ||
| { | ||
| type: 'turn_state_changed', | ||
| event_type: 'state:created', | ||
| new_value: same, | ||
| }, | ||
| 'sess-a', | ||
| ) | ||
| expect( | ||
| translate( | ||
| { | ||
| type: 'turn_state_changed', | ||
| event_type: 'state:updated', | ||
| new_value: same, | ||
| old_value: same, | ||
| }, | ||
| 'sess-a', | ||
| ), | ||
| ).toEqual([]) | ||
| }) | ||
|
|
||
| it('emits nothing when state leaves function_awaiting_approval (the orchestrator emits the matching function_execution_end)', () => { | ||
| const translate = createTurnStateTranslator() | ||
| translate( | ||
| { | ||
| type: 'turn_state_changed', | ||
| event_type: 'state:created', | ||
| new_value: { | ||
| state: 'function_awaiting_approval', | ||
| awaiting_approval: [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::shell', args: {} }, | ||
| ], | ||
| }, | ||
| }, | ||
| 'sess-a', | ||
| ) | ||
| expect( | ||
| translate( | ||
| { | ||
| type: 'turn_state_changed', | ||
| event_type: 'state:updated', | ||
| new_value: { state: 'function_execute', awaiting_approval: [] }, | ||
| old_value: { | ||
| state: 'function_awaiting_approval', | ||
| awaiting_approval: [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::shell', args: {} }, | ||
| ], | ||
| }, | ||
| }, | ||
| 'sess-a', | ||
| ), | ||
| ).toEqual([]) | ||
| }) | ||
|
|
||
| it('partitions mirrors by sessionId so two chats do not interfere', () => { | ||
| const translate = createTurnStateTranslator() | ||
| const pending = { | ||
| state: 'function_awaiting_approval', | ||
| awaiting_approval: [ | ||
| { function_call_id: 'fc-1', function_id: 'shell::shell', args: {} }, | ||
| ], | ||
| } | ||
| translate( | ||
| { type: 'turn_state_changed', event_type: 'state:created', new_value: pending }, | ||
| 'sess-a', | ||
| ) | ||
| expect( | ||
| translate( | ||
| { type: 'turn_state_changed', event_type: 'state:created', new_value: pending }, | ||
| 'sess-b', | ||
| ), | ||
| ).toHaveLength(1) | ||
| }) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we shouldn't call state directly from the frontend, it should be abstracted on the harness logic
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. Added
turn::get_state(harness-node/src/turn-orchestrator/get-state.ts) that wrapspersistence.loadRecord, and the console now calls it instead ofstate::getfor reload recovery. Schema and key layout stay inside the orchestrator. Pushed in 8292ad0.