fix(session): surface drain failures + check @mention skill permissions - #49988
holny wants to merge 2 commits into
Conversation
Sorry, something went wrong.
A terminal cause that mixes an interruption with a real failure settled as interrupted, dropping the error from the durable session.execution.failed event; only pure interruptions are deliberate stops now. The drain catch path also annotates its log with the same toSessionError envelope the durable event carries. Closes anomalyco#49740
Mentioned skills loaded through SessionPrompt.prepare without consulting the skill permission, so a denied skill's full body entered the model context via @mention while the skill tool path rejected it. Mentions now evaluate the same permission: deny rejects the prompt, ask admits the mention without the prepared body, allow injects as before. The server prompt endpoint maps the resulting BlockedError to a 400 on the skills field. Closes anomalyco#49891
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
Sorry, something went wrong.
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
Sorry, something went wrong.
|
The following comment was made by an LLM, it may be inaccurate: |
Sorry, something went wrong.
|
This pull request has been automatically closed because it was not updated to meet our contributing guidelines within the 2-hour window. Feel free to open a new pull request that follows our guidelines. |
Sorry, something went wrong.
Issue for this PR
Closes #49740 and closes #49891
Type of change
What does this PR do?
Two fixes in the session layer:
1. Drain failure classification (#49740)
terminal() used Cause.hasInterrupts to classify exits, which returns true when the cause contains any interrupt — even if a real failure is also present. Mixed interrupt+failure causes (e.g. inactivity eviction racing with a provider error) were silently classified as "interrupted" and never surfaced as durable failures.
Fixed to hasInterruptsOnly: only pure interruptions are deliberate stops; mixed causes settle as failed with the error envelope via toSessionError.
2. @mention skill permission check (#49891)
Skills attached via @mention in prompts bypassed the permission system entirely (prompt.ts went straight to skillService.get() + prepare()). The skill tool path correctly returns permission.rejected for denied skills, but @mention injected the full body regardless.
Mention-sourced skills now go through the same permission.ask() evaluation: deny → Permission.BlockedError, ask → attachment records the reference without injecting the body, allow → normal injection.
How did you verify your code works?