Repository navigation
feat(logs): persist the client's own session id on call-log rows - #17125
diegosouzapw merged 6 commits into
Conversation
5c4af8e to
3f37a78
Compare
|
Re-homed to |
New nullable column call_logs.client_session_id (migration 213, indexed) holding the id the client itself reports: opencode x-opencode-session, Claude Code x-claude-code-session-id or metadata.user_id.session_id, Codex session_id. resolveClientSessionId() already reads these for conversation tracking but the value was dropped after the fingerprint hash, so call-log rows could not be joined to the agent session that sent them. session_tag is unchanged and still holds OmniRoute's conversation id; the two columns are written side by side. The id is resolved at the attempt-logging seam from the clientRawRequest snapshot the attempt context already carries, so no frozen file changes. ensureCallLogsColumns back-fills the column and index for lineages that skip the migration, since the INSERT names the column on every write.
d3f38e5 to
d262bfa
Compare
|
Thanks for re-homing it. Rebased onto |
|
Thanks @AndresDiagoM, this is well built: small, follows the ttft_ms/session_tag pattern, never-throws resolver, bounded value, idempotent migration plus back-fill, and good tests (I ran your two suites plus call-logs-session-tag: 15/15 green, and check:migration-numbering / check:db-rules pass). Two things: (1) the last commit (docs: state 210 SQL migrations) edits AGENTS.md and llm.txt plus 66 llm.txt/README mirrors and .i18n-state.json; AGENTS.md/llm.txt are agent-instruction surfaces we can't merge without explicit owner approval, and the number is churn-prone. Could you drop that commit so the PR carries only the code, tests and changelog fragment? (2) Please merge the current release/v3.8.53 into the branch (15 commits behind, no conflicts today) and make sure migration 213 is still free at merge time. The remaining red checks are inherited from the base and should clear after the refresh. |
…logs-client-session-id
d262bfa to
721704f
Compare
|
Done, thanks for the review.
Re-ran locally on the merged branch: |
|
Refreshed: merged current Checked locally: |
…uest type The persistAttemptLogs context passes clientRawRequest with an endpoint field; widen RawRequestLike so the new unit test typechecks. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
adf90f8
into
cheaper-inference:release/v3.8.53
…igrations to 219/220 and share the session resolver Merge the updated cheaper-inference#14771 head (which carries release/v3.8.53) into feat/agent-sessions-project-attribution. The release now owns 214_call_logs_client_session_id and 215_team_cost_centers, so agent_sessions and usage_history.agent_session_id move to 219/220; the rename-compat entries point there and new 214/215 entries rehome databases that ran the previous build (covered by the renumbered migration test). usage_history inserts keep the release's billing_team_id column next to agent_session_id. chatCore passes agentContext to the streaming tail together with the release's Gemini cache arguments. agentContext now reads the client session id through resolveClientSessionId from conversationTracker (cheaper-inference#17125), so agent_sessions.client_session_id joins call_logs.client_session_id; x-omniroute-session-id still wins. Codex's id comes from its session_id header, so the Codex test now sends that header instead of x-codex-session-id. Count lines in AGENTS.md, llm.txt and docs/i18n take the release text; README.md states the real migration count. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
What
Persist the client's own session id on every call-log row, in a new nullable column
call_logs.client_session_id(migration 213, indexed).The value is what the client already sends and
resolveClientSessionId()already reads for conversation tracking:x-opencode-sessionheaderx-claude-code-session-idheader, ormetadata.user_id→session_idin the bodysession_idheadersession_tagis untouched: it keeps OmniRoute'sconv_…conversation id (or an explicitX-OmniRoute-Session-Id). The two columns are written side by side and answer different questions:session_tagis "which OmniRoute conversation",client_session_idis "which client session sent this".Why
Today you cannot join call-log rows to the agent session that produced them. The conversation id is minted by OmniRoute, and the client's own id is read on every request but dropped after the fingerprint hash.
That join matters when several agents share a combo. A priority combo falls back silently from step to step: the client only knows the combo name, and
combo_step_idis the only record of which step served. Withclient_session_id, a tool that knows a terminal's agent session (for example a terminal multiplexer's per-pane session id) can tell, per session, "this session's last three requests were served by step 2". It needs no client-side config and no per-session API key.Verified against a real client: opencode 2.0.27 sends
x-opencode-session: ses_…on every chat request. It is the same id the client reports for its own session.How
open-sse/handlers/chatCore/clientSessionId.ts(new, 40 lines) adaptsresolveClientSessionId()to theclientRawRequestsnapshot that reaches attempt logging. There,headersis a plain record (seechat/clientRawRequest.ts), not aHeadersinstance. It never throws and is bounded to 128 chars like the existing tracker.attemptLogging.tspassesclientSessionIdintosaveCallLog(+2 lines plus a type widening).clientRawRequestwas already in the attempt context, so no frozen file is touched:chat.ts,chatCore.tsanddb/core.tsare unchanged.callLogs.ts: normalize (toStringOrNull+ 128-char bound), INSERT column, and mapclientSessionIdinmapSummaryRow(list and by-id reads).schemaColumns.ts: back-fill the column and index for lineages that skip the migration, the same treatmentttft_msandsession_tagget, because the INSERT names the column on every write.213_call_logs_client_session_id.sql:ADD COLUMN … DEFAULT NULLplus an index. Rows written before it read NULL. The runner's existing duplicate-column tolerance covers a DB thatensureCallLogsColumns()healed first, and the test covers it too.Other call-log writers (embeddings, images, rejected admissions) are unchanged and write NULL. The value is metadata like
session_tag, so it is stored with no-log on as well. Nothing new is exported to log-export destinations.Tests
tests/unit/call-logs-client-session-id.test.ts(8): the resolver for each client and formetadata.user_id; case-insensitive header names; 128-char bound; null and never-throws on missing, blank, non-string or malformed input; persistence throughpersistAttemptLogsnext tosession_tag; NULL when the client sends none; stored under no-log;saveCallLogwithout the field writes NULL; the index exists.tests/unit/db/migration-213-call-logs-client-session-id.test.ts(2): upgrade from the prior schema keeps existing rows and reads NULL for them, and a re-run is a no-op. A DB that already has the column (healed at boot) still records 213 and gets the index.TDD: with only the resolver and migration in place, the three
persistAttemptLogstests failed on their assertions (actual: undefined, expected: 'ses_eae33138fffebuMYMAtH2Hs3Qs',actual: undefined, expected: null,actual: null). The wiring made them pass.Gates run locally:
check:migration-numbering,check:db-rulesandcheck:file-sizeall OK; the new suites pluscall-logs-session-tag,admission-rejection-call-log,migration-191,attempt-logging-extract-responses-idandcall-logs-id-collisionpass, except twocall-logs-id-collisionartifact tests that also fail on an unmodified tip on this machine (Windows, Node 24.11: the artifact worker thread cannot resolve the extensionlesssrc/lib/dataPathsimport), so they are environmental, not this change. ESLint is clean on the touched files except the pre-existingDeleteResultunused-type error incallLogs.ts, which is also on the tip.CI notes
release/v3.8.53at14cf503e(no conflicts); migration 213 is still the next free number on that tip. The earlier docs-count commit (AGENTS.md/llm.txt/ README mirrors /.i18n-state.json) was dropped per review, so the PR carries only the code, the tests, and the changelog fragment. The stated SQL migration count is therefore unchanged by this PR;check-docs-counts-syncwill report 210 vs the documented number until the owner bumps it.check:migration-numberingandcheck:db-rulesOK;call-logs-client-session-id,migration-213-call-logs-client-session-idandcall-logs-session-tag15/15 green.