Skip to content

fix: refuse saved limits before channel resume - #1780

Closed
jhgaylor wants to merge 1 commit into
fix/saved-allowance-queued-promptfrom
fix/saved-allowance-channel-resume
Closed

jhgaylor wants to merge 1 commit into
fix/saved-allowance-queued-promptfrom
fix/saved-allowance-channel-resume

Conversation

@jhgaylor

Copy link
Copy Markdown
Collaborator

Resuming a channel with an unsupported saved allowance could report success or replace caller tools before the prompt guard refused it. Check saved policy before returning the existing binding. Native, OpenAI-compatible and AG-UI requests now refuse without changing tools, conversation state or starting work. Read-only lookup for pending tool answers and explicit fresh-session rotation retain their existing behavior.

Stacked on #1779 (fix/saved-allowance-queued-prompt). Focused replacement for #1745/#1754; tracked in #1732. The API changes document HTTP422 for both chat dialects and preserve OpenAI's error envelope; generated contract/types match. SDK publication remains deferred (sdk-no-release); no version bump or client runtime change.

Validation: ten new tests across all three APIs, unsupported/malformed policy, suspended bindings, tenant isolation, absent/empty allowances, read-only lookup and fresh rotation. Seven fail on the parent. All 198 related tests, all four SDK contract checks and TypeScript typecheck pass. Full precommit passes 4,764 tests plus six doctests, zero failures; diff secret scan passes.

This is preflight; the transactional turn check remains authoritative against later policy writes. No new policy writer, ceilings, runtime controls or live enforcement proof is enabled. Direct boot recovery and remaining integration/live acceptance stay required.

Signed-off-by: Jake Gaylor <jhgaylor@gmail.com>

@BinaryBourbon BinaryBourbon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving.

Putting the check ahead of check_sandbox_api_resume/2 in the resume branch is the right position: the bug was that the OpenAI and AG-UI controllers replace caller_tools on the returned binding, so a refusal that arrives only at the prompt guard has already mutated the conversation. The test models that precisely — it stubs send_prompt to return the same refusal and then asserts Repo.reload!(conv) == before and refute_received :prompt_attempted. That is the assertion that would have caught the original bug, and it is testing the ordering rather than the return value.

Covering all three dialects in one matrix is worth the extra setup: the OpenAI envelope is a genuinely different shape (error.code / error.type) and it would be easy to get 422 right on the native route and leave that one returning a 500.

async: false here is correct and not gratuitous — the module writes :feature_flag_overrides into application env, and the on_exit restores the previous value including the "was not set at all" case.

Also checked that the fresh-rotation path is deliberately left alone: rotation creates a new conversation, so inheriting the old one's saved policy would be wrong, and the test asserts the new conversation has no allowance row.

The generated contract.json and openapi.ts hunks match the two responses: declarations, and no SDK version is bumped, so nothing publishes on merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:skip-sdk Explicitly allows an SDK surface change without releasing a new package version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants