Skip to content

[BUG]: SessionId accepts the empty string #800

Description

@justintime4tea

Summary

aura_events::SessionId is generated by string_newtype!, so SessionId::new("") is a session and "" deserializes into one. #794 moved ObserverId and CheckpointRef onto nonempty_string_newtype! and made RunId refuse the nil UUID, but left SessionId beside ToolCallId and ToolName, which may legitimately be anything.

Every store keyed by session inherits the hole. Today that is the skill-invocation store (#799): every client that sends a blank chat session id shares one skill log. With the session journal (#779) it is the journal: every run whose session id resolved to a blank appends to one stream, and the second writer trips the store's out-of-order refusal, failing its run.

#795 treats a blank chat session id as missing at the handler. That is the right rule, and it covers that one ingress. A2A context ids, Slack, the HITL scope, and stored approval records each construct a SessionId of their own, as will the runtime's start (#780) and POST /v1/runs (#785). A blank should be impossible to hold, not something each caller remembers to check.

Reproduction

  1. SessionId::new("") compiles and returns a session; serde_json::from_value::<SessionId>(json!("")) parses. Compare ObserverId::new(""), which is Err(EmptyId).
  2. On nightly, send two POST /v1/chat/completions requests with "metadata": {"chat_session_id": ""} that each invoke a skill tool: both record under one skill log, and each sees the other's rehydrated invocations ([BUG]: A blank chat session id is taken as given, so clients share a session #799).
  3. On [FEATURE]: A session's event journal and late attach #779's branch, open SessionJournal::open(SessionId::new(""), store) for two runs: they share one journal, and the second append fails with refusing seq 1 out of order.

Logs

n/a

Additional Context

Fix at the vocabulary level, where the identifier types live: generate SessionId with nonempty_string_newtype!, so new returns Result<Self, EmptyId>, TryFrom replaces From, and "" is refused on every path — construction and deserialization alike.

Each production site that builds a SessionId then decides what a blank means there, and the answer is the same everywhere: a blank names no session.

  • the chat handler: none given, fall through to the next source, mint one if none (fix(web-server): treat a blank chat session id as missing #795's rule)
  • the builder's Option<String> mapping, skill_tool, skill_rehydration, hitl::route: None
  • the ScopeRecord → AgentScope restore of a stored approval: a blank in an old record becomes None, not a decode failure
  • A2A context id and Slack: refuse or mint, per ingress

Blocked by PR #794, which this builds on (nonempty_string_newtype! and the identifier hardening it added); the dependency link points at #778, the issue that stack resolves, since GitHub links issues only. Part of the #578 runtime work insofar as #779 and #780 key everything by session.

Searched Issues

  • No similar issues found

Code of Conduct

  • I agree to follow this project's Code of Conduct

Activity

  1. added 3 commits that reference this issue on Oct 9, 2026
    7022782
    d23fd02
    17ddf89
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions