refactor(session): drop unused RUNTIME_SESSION_ID_META_KEYS alias - #509
Draft
KrasimirKralev wants to merge 1 commit into
Draft
refactor(session): drop unused RUNTIME_SESSION_ID_META_KEYS alias#509KrasimirKralev wants to merge 1 commit into
KrasimirKralev wants to merge 1 commit into
Conversation
The exported const RUNTIME_SESSION_ID_META_KEYS was a bare re-alias of AGENT_SESSION_ID_META_KEYS with no references anywhere in the repo (src, test, conformance, docs, scripts) and was not re-exported by any public entry (cli/flows/runtime). Remove it and its now-unused import binding; the live wrappers normalizeRuntimeSessionId/extractRuntimeSessionId and the AGENT_SESSION_ID_META_KEYS export (used by its own module + test) are untouched. Behavior-preserving.
Contributor
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P3
Contributor
|
Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 8:22 AM ET / 12:22 UTC. ClawSweeper reviewWhat this changesThe PR removes the unused internal session-metadata alias and its now-unneeded import while retaining the runtime session-ID wrapper functions. Merge readiness✅ Ready for maintainer review Keep open: current main still defines the alias, so this exact cleanup remains unmerged; the focused patch has no correctness finding and is ready for ordinary maintainer review once taken out of draft. Priority: P3 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherSession-ID helpers normalize and extract ACP agent session identifiers for the ACP client, CLI output, session management, runtime lifecycle, and persistence. The changed wrapper forwards to the canonical ACP helper and has no public entry-point export. flowchart LR
A[ACP response metadata] --> B[Canonical session-ID helper]
B --> C[Runtime session wrapper]
C --> D[ACP client]
C --> E[CLI and runtime lifecycle]
C --> F[Session persistence]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Land the one-file removal after the PR is marked ready, preserving the canonical ACP key export and the two live runtime wrapper functions. Do we have a high-confidence way to reproduce the issue? Not applicable: this PR removes an unreferenced internal alias rather than repairing a runtime symptom. Is this the best way to solve the issue? Yes. Removing the isolated alias is the narrowest maintainable cleanup because the canonical ACP key export and both live wrapper functions remain intact. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d4c16ab32154. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What Problem This Solves
src/session/runtime-session-id.tsexportedRUNTIME_SESSION_ID_META_KEYS, a bare re-alias ofAGENT_SESSION_ID_META_KEYS(export const RUNTIME_SESSION_ID_META_KEYS = AGENT_SESSION_ID_META_KEYS;). Nothing referenced it — a repo-wide search (src,test,conformance,docs,scripts) finds the symbol only at its own definition, and it is not part of the published API surface: the package buildssrc/cli.ts,src/flows.ts, andsrc/runtime.ts, and none of those graphs re-export it (noexport *barrel carries it). It is dead code that also forcedruntime-session-id.tsto importAGENT_SESSION_ID_META_KEYSsolely to alias it.Why This Change Was Made
Debt cleanup (family: dead-code removal). Remove the unused exported alias and its now-unnecessary import binding. The live sibling helpers in the same file —
normalizeRuntimeSessionId/extractRuntimeSessionId(the "runtime" wrappers actually consumed acrosssrc/acp/client.ts,src/cli/output/render.ts,src/session/persistence/*, etc.) — are untouched, as is theAGENT_SESSION_ID_META_KEYSexport insrc/acp/agent-session-id.js(still used internally atagent-session-id.ts:25and asserted bytest/agent-session-id.test.ts). Behavior-preserving, single production file, net −6 lines.User Impact
None. No runtime, CLI, config, or public-API behavior changes — the removed symbol was reachable from nothing (unexported from the public entry graphs, referenced nowhere). The
.d.tsoutput is unaffected because the alias was never in thecli/flows/runtimeentry surface.Evidence
Branched off current
main(2d735cf), Linux, Node 22,pnpm install --frozen-lockfile.git grep RUNTIME_SESSION_ID_META_KEYSreturns onlysrc/session/runtime-session-id.ts:7(the definition) before the change, and 0 hits after.pnpm run typecheck(tsc --noEmit) → exit 0.pnpm run lint(oxlint --type-aware --deny-warnings src …) → exit 0 (confirms the import cleanup leaves no unused binding).oxfmt --check src/session/runtime-session-id.ts→ clean.pnpm run build(tsdown,--dts) → exit 0.pnpm run build && build:test && node --test dist-test/test/*.test.js) → 935 pass, 0 fail on this branch. The removed symbol had no test, so the count matchesmain; behavior is preserved andtest/agent-session-id.test.ts(which asserts onAGENT_SESSION_ID_META_KEYS) still passes.Diff: +1 / −7, 1 file. Same shape as the merged dead-export cleanup #457.
AI-assisted contribution.
Generated by Claude Code