feat(agent-runtime): add session inspect and list commands#602
Merged
Conversation
Deploying corvus with
|
| Latest commit: |
687bb84
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://802b75a5.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-agent-runtime-session-b.corvus-42x.pages.dev |
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR implements Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PreExecution
participant Registry
participant SessionService
participant Memory
participant SQLite
rect rgba(100, 150, 255, 0.5)
Note over Client,SQLite: /session inspect flow
Client->>PreExecution: /session inspect
PreExecution->>Registry: evaluate_ingress(...)
Registry->>SessionService: SessionHandler → handle_session(context, "inspect")
SessionService->>Memory: get_session(session_id)
Memory->>SQLite: Query current session row
SQLite-->>Memory: Return session record
SessionService->>Memory: get_session_state_record(session_id)
Memory->>SQLite: Query slash-session state (if exists)
SQLite-->>Memory: Return state or None
SessionService->>Memory: get_session_snapshot(...) [resolve referenced IDs]
Memory->>SQLite: Query snapshot rows by ID
SQLite-->>Memory: Return snapshots with resume-capability
SessionService->>SessionService: Assemble inspect model<br/>(record + state + snapshots<br/>+ explicit gaps)
SessionService-->>Client: SessionCommandSuccess {<br/> SessionInspect {<br/> session, state, snapshots,<br/> gaps with codes<br/> }<br/>}
end
rect rgba(150, 200, 100, 0.5)
Note over Client,SQLite: /session list flow
Client->>PreExecution: /session list
PreExecution->>Registry: evaluate_ingress(...)
Registry->>SessionService: SessionHandler → handle_session(context, "list")
SessionService->>Memory: list_session_rows_for_scope(caller_scope)
Memory->>SQLite: Query sessions + state + snapshots<br/>WHERE token_hash = scope<br/>ORDER BY last_activity DESC
SQLite-->>Memory: Return rows with derived<br/>lifecycle + resumable
SessionService-->>Client: SessionCommandSuccess {<br/> SessionList {<br/> sessions: Vec<SessionListEntry><br/> }<br/>}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Dense service-level refactoring ( Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-20 to 2026-04-20 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/memory/sqlite.rs`:
- Around line 1638-1648: The JOIN to session_snapshots must be constrained to
the owning session and to compact snapshots so resumable isn't tripped by
another session's or non-compact snapshot; change the LEFT JOIN of
session_snapshots (alias snap) to include AND snap.session_id = s.id AND
snap.kind = 'compact' (or the exact compact kind enum used) so the CASE using
state.latest_compact_snapshot_id and snap.is_resume_capable only sees
same-session compact snapshots, and add a regression test creating session-a and
session-b where session-a.latest_compact_snapshot_id points at session-b's
resume-capable snapshot and assert session-a.resumable == false.
In `@clients/agent-runtime/src/session_commands/service.rs`:
- Around line 1203-1213: The inspect handler is returning raw snapshot.payload
via the SessionCommandInspectSnapshot construction (inside the
SessionCommandInspectSnapshotSlot variant); remove sensitive payload exposure by
omitting snapshot.payload from the returned structure or replacing it with a
redacted/size-bounded preview, and update the SessionCommandInspectSnapshot
definition in types.rs accordingly (remove the payload field or make it an
Option<RedactedPreview>). Ensure the code that builds
SessionCommandInspectSnapshot (the mapping of resolved.snapshot ->
SessionCommandInspectSnapshot) sets payload to None or the sanitized preview and
add any helper/redaction logic used to produce that preview.
- Around line 139-142: The current call to load_current_session_read_model(...)
resolves snapshots (slow and error-prone) even though
assemble_session_status(session_id, read_model.session, read_model.state) only
needs the raw session and state; change the service to load a lightweight read
model that does NOT resolve snapshots (either call an existing non-resolving
loader or add one, e.g., load_current_session_read_model_without_snapshots or
load_session_summary_no_snapshots) and pass its session and state into
assemble_session_status so /session status remains fast and unaffected by
snapshot read errors.
In `@openspec/changes/archive/2026-04-19-slash-session-inspect/design.md`:
- Around line 176-196: The design doc's handler contract for
SessionCommandService::handle_session(session_id, raw_args) and the example
mapping (raw_args => SessionHelp/Status/Inspect/InvalidArguments) is stale after
the "list" seam widening; update the text to either (a) explicitly qualify this
contract as "pre-list/inspect-slice-only" or (b) document the new
behavior/signature (e.g., that /session list is routed through the same service
and requires caller context, and that handle_session must accept the additional
caller context or return a distinct Branch for List) and adjust the examples so
"/session list" is no longer shown as InvalidArguments but described according
to the new routing/contract; reference SessionCommandService::handle_session,
raw_args, SessionHelp, SessionStatus, SessionInspect, SessionList (or the chosen
new branch), and InvalidArguments when making the change.
In
`@openspec/changes/archive/2026-04-19-slash-session-inspect/specs/sessions/spec.md`:
- Around line 28-33: The `/session status` recommendation matrix is missing the
case for a suspended session with no latest compact snapshot reference; add a
rule so the `/session status` result still returns exactly one actionable
recommendation in that state by recommending `/compact` for a suspended session
when "latest compact snapshot reference" is absent, and keep the existing rules
for active-without-snapshot => `/compact`, active-with-snapshot => `/suspend`,
suspended-with-snapshot => `/resume`, and withholding recommendations when no
authoritative current session record exists.
In
`@openspec/changes/archive/2026-04-19-slash-session-inspect/specs/slash-command-registry/spec.md`:
- Around line 40-57: The spec's unsupported-subcommand scenario incorrectly uses
`/session list` which is now a supported raw-args branch; update the scenario so
the example is truly unsupported (e.g., change `/session list` to `/session
archive`) or alternatively mark `list` as a recognized subcommand in the
`/session` family; ensure the text that references routing through
pre_execution::evaluate_ingress(...) and the registry resolution of the
canonical `/session` command reflects this change so `/session archive` (or the
newly recognized `list`) behaves as described.
In `@openspec/changes/archive/2026-04-19-slash-session-inspect/verify-report.md`:
- Around line 76-82: Update the verification text to limit the "inspect-only"
claims to the inspect feature: change the sentence that states
handle_session(...) accepts only "", status, and inspect to explicitly say
"handle_session(...) accepts only "", status, and inspect for the inspect slice;
/session list is implemented separately and adds a caller-scoped memory list
contract." Also update the Memory/sqlite scope containment line to clarify "No
memory contract or persistence expansion was introduced for the inspect
functionality; /session list introduces a separate caller-scoped list contract."
Ensure these edits reference the existing symbols handle_session(...) and
load_current_session_read_model(...) so readers know the scope applies only to
the inspect slice.
In `@openspec/changes/archive/2026-04-19-slash-session-list/proposal.md`:
- Around line 9-20: Add missing blank lines after the "In Scope" and "Out of
Scope" headings to satisfy markdownlint (insert an empty line after those
heading lines), and reword the awkward phrase "requires balanced" (around the
"balanced output" sentence near the minimal row shape / lines ~84-89) to a
clearer phrasing such as "Define balanced output: concise human-readable summary
plus structured row data" so the wording is natural; update the heading spacing
and the sentence in the section describing the balanced output (the lines
mentioning minimal row shape and "balanced") accordingly.
In
`@openspec/changes/archive/2026-04-19-slash-session-list/specs/sessions/spec.md`:
- Around line 45-51: The example's expected stable tiebreak order conflicts with
the spec's defined sort key "last_activity DESC, id DESC": update the scenario
for `/session list` so the expected relative order for equal `last_activity`
matches `id DESC` (i.e., `sess-b` before `sess-a`) or remove explicit names and
state "a stable secondary ordering (id DESC) is used"; change the wording in the
Scenario title or THEN clause to reflect `id DESC` as the stable tiebreaker and
ensure references to `sess-a`/`sess-b` align with that rule.
In `@openspec/changes/archive/2026-04-19-slash-session-list/verify-report.md`:
- Line 101: In the table row starting with "Resumable derived from authoritative
current capability" update the awkward phrase "requires suspended lifecycle" to
clearer wording such as "requires the lifecycle to be suspended" (or "requires
that the lifecycle be suspended") so the third-cell description reads: "Query
requires the lifecycle to be suspended, latest compact snapshot reference,
existing snapshot row, and `is_resume_capable = 1`."
In `@openspec/specs/sessions/spec.md`:
- Around line 384-390: Update the scenario "Stable tiebreaker preserves repeated
ordering for equal activity timestamps" to match the implemented stable
secondary rule `id DESC`: change any example ordering that shows `sess-a` before
`sess-b` so that `sess-b` is listed before `sess-a` in the GIVEN and THEN lines,
and explicitly mention the secondary ordering rule `id DESC` in the scenario
text (referencing the scenario header and the session ids `sess-a` and `sess-b`
so the example aligns with the implemented tie-breaker).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c4983d5f-8532-4fb8-b24d-81dd7446d44b
📒 Files selected for processing (26)
.github/dependabot.ymlclients/agent-runtime/crates/corvus-traits/src/memory.rsclients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/memory/traits.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/session_commands/service.rsclients/agent-runtime/src/session_commands/types.rsopenspec/changes/archive/2026-04-19-slash-session-inspect/design.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/proposal.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/state.yamlopenspec/changes/archive/2026-04-19-slash-session-inspect/tasks.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/verify-report.mdopenspec/changes/archive/2026-04-19-slash-session-list/design.mdopenspec/changes/archive/2026-04-19-slash-session-list/proposal.mdopenspec/changes/archive/2026-04-19-slash-session-list/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/state.yamlopenspec/changes/archive/2026-04-19-slash-session-list/tasks.mdopenspec/changes/archive/2026-04-19-slash-session-list/verify-report.mdopenspec/specs/sessions/spec.mdopenspec/specs/slash-command-registry/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: submit-gradle
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (5)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/archive/2026-04-19-slash-session-list/state.yamlclients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/pre_execution/mod.rsopenspec/changes/archive/2026-04-19-slash-session-inspect/state.yamlclients/agent-runtime/src/memory/traits.rsopenspec/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/verify-report.mdclients/agent-runtime/src/session_commands/registry.rsopenspec/changes/archive/2026-04-19-slash-session-inspect/design.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/proposal.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/specs/slash-command-registry/spec.mdclients/agent-runtime/crates/corvus-traits/src/memory.rsopenspec/changes/archive/2026-04-19-slash-session-list/proposal.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/tasks.mdclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/types.rsopenspec/changes/archive/2026-04-19-slash-session-list/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/tasks.mdopenspec/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/design.mdclients/agent-runtime/src/session_commands/service.rsopenspec/changes/archive/2026-04-19-slash-session-inspect/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/verify-report.mdopenspec/changes/archive/2026-04-19-slash-session-list/specs/slash-command-registry/spec.md
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/memory/traits.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/types.rsclients/agent-runtime/src/session_commands/service.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/memory/traits.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/crates/corvus-traits/src/memory.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/types.rsclients/agent-runtime/src/session_commands/service.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/memory/mod.rsclients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/memory/traits.rsclients/agent-runtime/src/session_commands/registry.rsclients/agent-runtime/crates/corvus-traits/src/memory.rsclients/agent-runtime/src/memory/sqlite.rsclients/agent-runtime/src/session_commands/mod.rsclients/agent-runtime/src/session_commands/types.rsclients/agent-runtime/src/session_commands/service.rs
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/verify-report.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/design.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/proposal.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/specs/slash-command-registry/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/proposal.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/tasks.mdopenspec/changes/archive/2026-04-19-slash-session-list/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/tasks.mdopenspec/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-list/design.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/verify-report.mdopenspec/changes/archive/2026-04-19-slash-session-list/specs/slash-command-registry/spec.md
🧠 Learnings (7)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/memory/traits.rsclients/agent-runtime/src/session_commands/mod.rsopenspec/changes/archive/2026-04-19-slash-session-list/design.mdclients/agent-runtime/src/session_commands/service.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/agent-runtime/src/pre_execution/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
clients/agent-runtime/src/pre_execution/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/agent-runtime/src/pre_execution/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/pre_execution/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/agent-runtime/src/pre_execution/mod.rsclients/agent-runtime/src/session_commands/service.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/src/memory/sqlite.rs
🪛 LanguageTool
openspec/changes/archive/2026-04-19-slash-session-list/verify-report.md
[style] ~101-~101: The double modal “requires suspended” is nonstandard (only accepted in certain dialects). Consider “to be suspended”.
Context: ...ent capability | ✅ Yes | Query requires suspended lifecycle, latest compact snapshot refe...
(NEEDS_FIXED)
openspec/changes/archive/2026-04-19-slash-session-inspect/proposal.md
[style] ~13-~13: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nced snapshot details when available. - Define partial-data behavior so inspect return...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/archive/2026-04-19-slash-session-list/proposal.md
[style] ~88-~88: The double modal “requires balanced” is nonstandard (only accepted in certain dialects). Consider “to be balanced”.
Context: ...e`. - [ ] The listing contract requires balanced human + structured output and determini...
(NEEDS_FIXED)
openspec/changes/archive/2026-04-19-slash-session-list/design.md
[style] ~49-~49: Consider removing “of” to be more concise
Context: ...**: Report resumable = true only when all of the following are true for the listed sessi...
(ALL_OF_THE)
[style] ~70-~70: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... data only - Return human-readable text only - Reuse /resume message formatting *...
(ADVERB_REPETITION_PREMIUM)
openspec/changes/archive/2026-04-19-slash-session-inspect/verify-report.md
[style] ~82-~82: The double modal “needed read” is nonstandard (only accepted in certain dialects). Consider “to be read”.
Context: ...d SQLite methods already provide needed read APIs; no new memory contract or persist...
(NEEDS_FIXED)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-19-slash-session-list/proposal.md
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (12)
.github/dependabot.yml (1)
53-57: Formatting-only change looks correct.
patternsvalues are unchanged and still resolve to string arrays, so Dependabot grouping behavior should remain the same.openspec/changes/archive/2026-04-19-slash-session-list/state.yaml (1)
1-13: Archive state looks consistent.The lifecycle fields correctly mark the change archived with no next phase.
openspec/changes/archive/2026-04-19-slash-session-inspect/state.yaml (1)
1-13: Archive state looks consistent.The lifecycle fields correctly mark the change archived with no next phase.
clients/agent-runtime/src/memory/mod.rs (1)
29-29: Public export aligns with the new session-list contract.
SessionListEntryis now available through the memory module alongside related session types.clients/agent-runtime/src/memory/traits.rs (1)
8-10: Compatibility shim is updated correctly.The new
SessionListEntrycontract is re-exported with the rest of the memory trait types.clients/agent-runtime/src/pre_execution/mod.rs (1)
328-390: Good seam coverage for the expanded/sessionfamily.The tests verify
inspect,list, and unsupported trailing text all stay routed through the canonical/sessionhandler boundary.openspec/specs/slash-command-registry/spec.md (1)
143-149: Spec aligns with canonical/sessionrouting.The raw-args treatment for
status,inspect, andlistmatches the registry/service routing shown in the Rust changes.Also applies to: 166-188, 198-201, 217-220
clients/agent-runtime/src/session_commands/registry.rs (1)
259-260: Canonical family routing is preserved.The descriptor, handler seam, and tests consistently keep
inspect/listas raw-args forms under/session, not standalone registrations.Also applies to: 427-427, 840-842, 1136-1220
clients/agent-runtime/crates/corvus-traits/src/memory.rs (1)
357-363: LGTM — minimal list row and backend gating are consistent.The new row contract stays narrow, and the default trait method preserves the existing explicit unsupported-backend behavior.
Also applies to: 572-579
openspec/changes/archive/2026-04-19-slash-session-list/specs/slash-command-registry/spec.md (1)
5-49: LGTM — canonical/sessionrouting is preserved.The spec clearly keeps
/session listas raw args under/sessionand routes supported/unsupported family forms through the shared pre-execution seam.openspec/changes/archive/2026-04-19-slash-session-inspect/proposal.md (1)
1-75: LGTM — inspect scope is clear and bounded.The proposal keeps inspect current-session-only/read-only and correctly separates this slice from the list archive.
clients/agent-runtime/src/session_commands/mod.rs (1)
12-24: LGTM — public exports cover the new inspect payload types.The re-export surface matches the structured inspect additions without changing runtime behavior.
Comment on lines
+1638
to
+1648
| CASE | ||
| WHEN state.lifecycle_state = 'suspended' | ||
| AND state.latest_compact_snapshot_id IS NOT NULL | ||
| AND snap.id IS NOT NULL | ||
| AND snap.is_resume_capable = 1 | ||
| THEN 1 | ||
| ELSE 0 | ||
| END AS resumable | ||
| FROM sessions s | ||
| LEFT JOIN session_state state ON state.session_id = s.id | ||
| LEFT JOIN session_snapshots snap ON snap.id = state.latest_compact_snapshot_id |
Contributor
There was a problem hiding this comment.
Validate snapshot ownership and kind before marking a row resumable.
Line 1648 joins snapshots only by id, so a mismatched latest_compact_snapshot_id can make this session appear resumable from another session’s resume-capable snapshot or from a non-compact snapshot. Gate the join to the same session and compact kind before Line 1642 can satisfy the resumable CASE.
🛡️ Proposed fix
- LEFT JOIN session_snapshots snap ON snap.id = state.latest_compact_snapshot_id
+ LEFT JOIN session_snapshots snap
+ ON snap.id = state.latest_compact_snapshot_id
+ AND snap.session_id = s.id
+ AND snap.snapshot_kind = 'compact'Please add a regression test where session-a points at a resume-capable snapshot owned by session-b and assert session-a.resumable == false. As per coding guidelines, Security first, performance second.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| CASE | |
| WHEN state.lifecycle_state = 'suspended' | |
| AND state.latest_compact_snapshot_id IS NOT NULL | |
| AND snap.id IS NOT NULL | |
| AND snap.is_resume_capable = 1 | |
| THEN 1 | |
| ELSE 0 | |
| END AS resumable | |
| FROM sessions s | |
| LEFT JOIN session_state state ON state.session_id = s.id | |
| LEFT JOIN session_snapshots snap ON snap.id = state.latest_compact_snapshot_id | |
| CASE | |
| WHEN state.lifecycle_state = 'suspended' | |
| AND state.latest_compact_snapshot_id IS NOT NULL | |
| AND snap.id IS NOT NULL | |
| AND snap.is_resume_capable = 1 | |
| THEN 1 | |
| ELSE 0 | |
| END AS resumable | |
| FROM sessions s | |
| LEFT JOIN session_state state ON state.session_id = s.id | |
| LEFT JOIN session_snapshots snap | |
| ON snap.id = state.latest_compact_snapshot_id | |
| AND snap.session_id = s.id | |
| AND snap.snapshot_kind = 'compact' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/memory/sqlite.rs` around lines 1638 - 1648, The
JOIN to session_snapshots must be constrained to the owning session and to
compact snapshots so resumable isn't tripped by another session's or non-compact
snapshot; change the LEFT JOIN of session_snapshots (alias snap) to include AND
snap.session_id = s.id AND snap.kind = 'compact' (or the exact compact kind enum
used) so the CASE using state.latest_compact_snapshot_id and
snap.is_resume_capable only sees same-session compact snapshots, and add a
regression test creating session-a and session-b where
session-a.latest_compact_snapshot_id points at session-b's resume-capable
snapshot and assert session-a.resumable == false.
Comment on lines
+139
to
+142
| let read_model = self | ||
| .load_current_session_read_model("/session", session_id) | ||
| .await?; | ||
| let status = assemble_session_status(session_id, read_model.session, read_model.state); |
Contributor
There was a problem hiding this comment.
Keep /session status from resolving snapshots.
load_current_session_read_model() resolves referenced snapshots, but assemble_session_status() only uses session and state. This makes the compact status path slower and can fail due to snapshot read errors unrelated to status.
Proposed fix
- let read_model = self
- .load_current_session_read_model("/session", session_id)
- .await?;
- let status = assemble_session_status(session_id, read_model.session, read_model.state);
+ let session = self
+ .memory
+ .get_session(session_id)
+ .await
+ .map_err(|error| self.map_storage_error("/session", Some(session_id), error))?;
+ let state = if session.is_some() {
+ self.get_session_state_optional("/session", session_id).await?
+ } else {
+ None
+ };
+ let status = assemble_session_status(session_id, session, state);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/session_commands/service.rs` around lines 139 -
142, The current call to load_current_session_read_model(...) resolves snapshots
(slow and error-prone) even though assemble_session_status(session_id,
read_model.session, read_model.state) only needs the raw session and state;
change the service to load a lightweight read model that does NOT resolve
snapshots (either call an existing non-resolving loader or add one, e.g.,
load_current_session_read_model_without_snapshots or
load_session_summary_no_snapshots) and pass its session and state into
assemble_session_status so /session status remains fast and unaffected by
snapshot read errors.
Comment on lines
+1203
to
+1213
| SessionCommandInspectSnapshotSlot { | ||
| reference_id: resolved.reference_id, | ||
| snapshot: resolved | ||
| .snapshot | ||
| .map(|snapshot| SessionCommandInspectSnapshot { | ||
| snapshot_id: snapshot.id, | ||
| kind: snapshot.kind, | ||
| created_at: snapshot.created_at, | ||
| resume_capable: snapshot.resume_capable, | ||
| payload: snapshot.payload, | ||
| }), |
Contributor
There was a problem hiding this comment.
Do not return raw snapshot payloads from inspect.
snapshot.payload can contain transcript-derived resume context and operator notes from compact snapshots, so exposing it in structured /session inspect data can leak sensitive session content beyond the concise inspection summary. Return metadata only, or an explicitly redacted preview. As per coding guidelines, “Security first, performance second” and avoid exposing sensitive payloads.
Proposed direction
snapshot: resolved
.snapshot
.map(|snapshot| SessionCommandInspectSnapshot {
snapshot_id: snapshot.id,
kind: snapshot.kind,
created_at: snapshot.created_at,
resume_capable: snapshot.resume_capable,
- payload: snapshot.payload,
}),Also remove the payload field from SessionCommandInspectSnapshot in types.rs, or replace it with a redacted/size-bounded preview.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/agent-runtime/src/session_commands/service.rs` around lines 1203 -
1213, The inspect handler is returning raw snapshot.payload via the
SessionCommandInspectSnapshot construction (inside the
SessionCommandInspectSnapshotSlot variant); remove sensitive payload exposure by
omitting snapshot.payload from the returned structure or replacing it with a
redacted/size-bounded preview, and update the SessionCommandInspectSnapshot
definition in types.rs accordingly (remove the payload field or make it an
Option<RedactedPreview>). Ensure the code that builds
SessionCommandInspectSnapshot (the mapping of resolved.snapshot ->
SessionCommandInspectSnapshot) sets payload to None or the sanitized preview and
add any helper/redaction logic used to produce that preview.
Comment on lines
+176
to
+196
| ```rust | ||
| impl<'a> SessionCommandService<'a> { | ||
| pub async fn handle_session(&self, session_id: &str, raw_args: &str) -> SessionCommandOutcome; | ||
| } | ||
| ``` | ||
|
|
||
| Expected branch behavior: | ||
|
|
||
| ```text | ||
| raw_args == "" => success(SessionHelp) | ||
| raw_args == "status" => success(SessionStatus) | ||
| raw_args == "inspect" => success(SessionInspect) | ||
| anything else => failure(InvalidArguments) | ||
| ``` | ||
|
|
||
| Extra trailing tokens remain invalid for this slice: | ||
|
|
||
| ```text | ||
| /session inspect extra => InvalidArguments | ||
| /session list => InvalidArguments | ||
| ``` |
Contributor
There was a problem hiding this comment.
Update or qualify the handler contract after the list seam widening.
The final PR routes /session list through the same service boundary and requires caller context, so this inspect design’s handle_session(session_id, raw_args) signature and /session list => InvalidArguments example are stale unless explicitly marked as pre-list/inspect-slice-only.
📝 Proposed direction
- pub async fn handle_session(&self, session_id: &str, raw_args: &str) -> SessionCommandOutcome;
+ pub async fn handle_session(&self, context: &CommandContext, raw_args: &str) -> SessionCommandOutcome;
@@
/session inspect extra => InvalidArguments
-/session list => InvalidArguments
+// `/session list` is specified by the separate slash-session-list slice.As per coding guidelines, **/*.md: Verify technical accuracy and that docs stay aligned with code changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-04-19-slash-session-inspect/design.md` around
lines 176 - 196, The design doc's handler contract for
SessionCommandService::handle_session(session_id, raw_args) and the example
mapping (raw_args => SessionHelp/Status/Inspect/InvalidArguments) is stale after
the "list" seam widening; update the text to either (a) explicitly qualify this
contract as "pre-list/inspect-slice-only" or (b) document the new
behavior/signature (e.g., that /session list is routed through the same service
and requires caller context, and that handle_session must accept the additional
caller context or return a distinct Branch for List) and adjust the examples so
"/session list" is no longer shown as InvalidArguments but described according
to the new routing/contract; reference SessionCommandService::handle_session,
raw_args, SessionHelp, SessionStatus, SessionInspect, SessionList (or the chosen
new branch), and InvalidArguments when making the change.
Comment on lines
+28
to
+33
| The `/session status` result MUST also include exactly one actionable recommendation derived from the current session state: | ||
| - it MUST recommend `/compact` when the current session is active and has no latest compact snapshot reference; | ||
| - it MUST recommend `/suspend` when the current session is active and already has a latest compact snapshot reference; | ||
| - it MUST recommend `/resume` when the current session is suspended and has a latest compact snapshot reference; and | ||
| - it MUST withhold lifecycle-command recommendations when no authoritative current session record exists. | ||
|
|
Contributor
There was a problem hiding this comment.
Recommendation contract is internally inconsistent for one valid state.
You require “exactly one actionable recommendation,” but no rule is defined for a suspended session with no latest compact snapshot reference. That leaves behavior undefined and can cause divergent implementations.
Proposed spec fix
The `/session status` result MUST also include exactly one actionable recommendation derived from the current session state:
- it MUST recommend `/compact` when the current session is active and has no latest compact snapshot reference;
- it MUST recommend `/suspend` when the current session is active and already has a latest compact snapshot reference;
- it MUST recommend `/resume` when the current session is suspended and has a latest compact snapshot reference; and
+- it MUST recommend `/compact` when the current session is suspended and has no latest compact snapshot reference; and
- it MUST withhold lifecycle-command recommendations when no authoritative current session record exists.As per coding guidelines "**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-19-slash-session-inspect/specs/sessions/spec.md`
around lines 28 - 33, The `/session status` recommendation matrix is missing the
case for a suspended session with no latest compact snapshot reference; add a
rule so the `/session status` result still returns exactly one actionable
recommendation in that state by recommending `/compact` for a suspended session
when "latest compact snapshot reference" is absent, and keep the existing rules
for active-without-snapshot => `/compact`, active-with-snapshot => `/suspend`,
suspended-with-snapshot => `/resume`, and withholding recommendations when no
authoritative current session record exists.
Comment on lines
+76
to
+82
| | Scope boundary: no list/browse/target-session expansion | ✅ Implemented | `handle_session(...)` accepts only `""`, `status`, and `inspect`; anything else returns `InvalidArguments`. | | ||
| | Current-session-only, read-only inspect loader | ✅ Implemented | `load_current_session_read_model(...)` calls `get_session(session_id)` first, then optional `get_session_state_record(session_id)`, then only referenced `get_session_snapshot(...)` lookups. | | ||
| | Human + structured output from same assembled model | ✅ Implemented | `handle_session_inspect(...)` builds one `SessionCommandSessionInspect` value, then `format_session_inspect_message(&inspect)` and `SessionCommandSuccessData::SessionInspect { inspect }` both derive from that same object. | | ||
| | Explicit partial-data gap reporting | ✅ Implemented | Gap codes include `SlashSessionStateMissing`, `SnapshotUnavailableWithoutState`, `ReferencedSnapshotMissing`, `ReferencedSnapshotOwnershipMismatch`, and `ReferencedSnapshotKindMismatch`; message rendering prints those same gap details. | | ||
| | Non-invented state behavior | ✅ Implemented | Unknown session returns `current_session_known = false` with no state/snapshots; missing state produces explicit gaps instead of default lifecycle/snapshot facts. | | ||
| | `/session status` remains compact | ✅ Implemented | Status path still uses dedicated compact payload/message formatter and remains separate from inspect via a distinct `SessionInspect` success variant. | | ||
| | Memory/sqlite scope containment | ✅ Implemented | Existing memory trait exports and SQLite methods already provide needed read APIs; no new memory contract or persistence expansion was introduced. | |
Contributor
There was a problem hiding this comment.
Qualify inspect-only verification claims now that /session list ships in the same PR.
Line 76 says handle_session(...) accepts only "", status, and inspect, and Line 82 says no memory contract expansion was introduced. In the final tree, /session list is supported and adds a caller-scoped memory list contract, so this report should scope those claims to the inspect slice only.
📝 Proposed wording
-| Scope boundary: no list/browse/target-session expansion | ✅ Implemented | `handle_session(...)` accepts only `""`, `status`, and `inspect`; anything else returns `InvalidArguments`. |
+| Scope boundary: inspect remains current-session-only | ✅ Implemented | The `inspect` branch accepts no target-session arguments; `/session list` is covered by the separate `slash-session-list` archive. |
@@
-| Memory/sqlite scope containment | ✅ Implemented | Existing memory trait exports and SQLite methods already provide needed read APIs; no new memory contract or persistence expansion was introduced. |
+| Inspect memory/sqlite scope containment | ✅ Implemented | Inspect uses existing read APIs; the separate `/session list` slice introduces its own caller-scoped listing contract. |As per coding guidelines, **/*.md: Verify technical accuracy and that docs stay aligned with code changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | Scope boundary: no list/browse/target-session expansion | ✅ Implemented | `handle_session(...)` accepts only `""`, `status`, and `inspect`; anything else returns `InvalidArguments`. | | |
| | Current-session-only, read-only inspect loader | ✅ Implemented | `load_current_session_read_model(...)` calls `get_session(session_id)` first, then optional `get_session_state_record(session_id)`, then only referenced `get_session_snapshot(...)` lookups. | | |
| | Human + structured output from same assembled model | ✅ Implemented | `handle_session_inspect(...)` builds one `SessionCommandSessionInspect` value, then `format_session_inspect_message(&inspect)` and `SessionCommandSuccessData::SessionInspect { inspect }` both derive from that same object. | | |
| | Explicit partial-data gap reporting | ✅ Implemented | Gap codes include `SlashSessionStateMissing`, `SnapshotUnavailableWithoutState`, `ReferencedSnapshotMissing`, `ReferencedSnapshotOwnershipMismatch`, and `ReferencedSnapshotKindMismatch`; message rendering prints those same gap details. | | |
| | Non-invented state behavior | ✅ Implemented | Unknown session returns `current_session_known = false` with no state/snapshots; missing state produces explicit gaps instead of default lifecycle/snapshot facts. | | |
| | `/session status` remains compact | ✅ Implemented | Status path still uses dedicated compact payload/message formatter and remains separate from inspect via a distinct `SessionInspect` success variant. | | |
| | Memory/sqlite scope containment | ✅ Implemented | Existing memory trait exports and SQLite methods already provide needed read APIs; no new memory contract or persistence expansion was introduced. | | |
| | Scope boundary: inspect remains current-session-only | ✅ Implemented | The `inspect` branch accepts no target-session arguments; `/session list` is covered by the separate `slash-session-list` archive. | | |
| | Current-session-only, read-only inspect loader | ✅ Implemented | `load_current_session_read_model(...)` calls `get_session(session_id)` first, then optional `get_session_state_record(session_id)`, then only referenced `get_session_snapshot(...)` lookups. | | |
| | Human + structured output from same assembled model | ✅ Implemented | `handle_session_inspect(...)` builds one `SessionCommandSessionInspect` value, then `format_session_inspect_message(&inspect)` and `SessionCommandSuccessData::SessionInspect { inspect }` both derive from that same object. | | |
| | Explicit partial-data gap reporting | ✅ Implemented | Gap codes include `SlashSessionStateMissing`, `SnapshotUnavailableWithoutState`, `ReferencedSnapshotMissing`, `ReferencedSnapshotOwnershipMismatch`, and `ReferencedSnapshotKindMismatch`; message rendering prints those same gap details. | | |
| | Non-invented state behavior | ✅ Implemented | Unknown session returns `current_session_known = false` with no state/snapshots; missing state produces explicit gaps instead of default lifecycle/snapshot facts. | | |
| | `/session status` remains compact | ✅ Implemented | Status path still uses dedicated compact payload/message formatter and remains separate from inspect via a distinct `SessionInspect` success variant. | | |
| | Inspect memory/sqlite scope containment | ✅ Implemented | Inspect uses existing read APIs; the separate `/session list` slice introduces its own caller-scoped listing contract. | |
🧰 Tools
🪛 LanguageTool
[style] ~82-~82: The double modal “needed read” is nonstandard (only accepted in certain dialects). Consider “to be read”.
Context: ...d SQLite methods already provide needed read APIs; no new memory contract or persist...
(NEEDS_FIXED)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-04-19-slash-session-inspect/verify-report.md`
around lines 76 - 82, Update the verification text to limit the "inspect-only"
claims to the inspect feature: change the sentence that states
handle_session(...) accepts only "", status, and inspect to explicitly say
"handle_session(...) accepts only "", status, and inspect for the inspect slice;
/session list is implemented separately and adds a caller-scoped memory list
contract." Also update the Memory/sqlite scope containment line to clarify "No
memory contract or persistence expansion was introduced for the inspect
functionality; /session list introduces a separate caller-scoped list contract."
Ensure these edits reference the existing symbols handle_session(...) and
load_current_session_read_model(...) so readers know the scope applies only to
the inspect slice.
Comment on lines
+9
to
+20
| ## Scope | ||
|
|
||
| ### In Scope | ||
| - Add `/session list` as a supported raw-args subcommand of canonical `/session`. | ||
| - Keep `/session list` read-only and caller-scoped, listing only sessions visible to the current caller scope. | ||
| - Define balanced output: concise human-readable summary plus structured row data. | ||
| - Define minimal row shape only: `id`, `last_activity`, `lifecycle`, `resumable`. | ||
| - Require deterministic ordering by `last_activity DESC` with an explicit stable tiebreaker. | ||
| - Make the command-context seam change explicit so the handler can enforce caller-scoped visibility. | ||
| - Extend the `slash-command-registry` and `sessions` OpenSpec domains for this slice. | ||
|
|
||
| ### Out of Scope |
Contributor
There was a problem hiding this comment.
Fix the markdownlint warnings before archiving.
Lines 11 and 20 need a blank line after the heading, and Line 88 should avoid the awkward “requires balanced” wording.
📝 Proposed cleanup
### In Scope
+
- Add `/session list` as a supported raw-args subcommand of canonical `/session`.
@@
### Out of Scope
+
- Admin/global session listing.
@@
-- [ ] The listing contract requires balanced human + structured output and deterministic ordering by `last_activity DESC` with a stable tiebreaker.
+- [ ] The listing contract requires balanced human-readable and structured output and deterministic ordering by `last_activity DESC` with a stable tiebreaker.Also applies to: 84-89
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 11-11: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 20-20: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-04-19-slash-session-list/proposal.md` around
lines 9 - 20, Add missing blank lines after the "In Scope" and "Out of Scope"
headings to satisfy markdownlint (insert an empty line after those heading
lines), and reword the awkward phrase "requires balanced" (around the "balanced
output" sentence near the minimal row shape / lines ~84-89) to a clearer
phrasing such as "Define balanced output: concise human-readable summary plus
structured row data" so the wording is natural; update the heading spacing and
the sentence in the section describing the balanced output (the lines mentioning
minimal row shape and "balanced") accordingly.
Comment on lines
+45
to
+51
| #### Scenario: Stable tiebreaker preserves repeated ordering for equal activity timestamps | ||
|
|
||
| - GIVEN the current caller scope is authorized to view sessions `sess-a` and `sess-b` | ||
| - AND both sessions have the same authoritative `last_activity` value | ||
| - WHEN the user runs `/session list` multiple times without any underlying session changes | ||
| - THEN the system MUST return `sess-a` and `sess-b` in the same relative order on each execution | ||
| - AND that ordering MUST be produced by a stable secondary ordering rule. |
Contributor
There was a problem hiding this comment.
Align the tie-break example with id DESC.
The design/tasks call for last_activity DESC, id DESC; for sess-a and sess-b, that returns sess-b before sess-a. Update the expected order or avoid naming a specific order here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/archive/2026-04-19-slash-session-list/specs/sessions/spec.md`
around lines 45 - 51, The example's expected stable tiebreak order conflicts
with the spec's defined sort key "last_activity DESC, id DESC": update the
scenario for `/session list` so the expected relative order for equal
`last_activity` matches `id DESC` (i.e., `sess-b` before `sess-a`) or remove
explicit names and state "a stable secondary ordering (id DESC) is used"; change
the wording in the Scenario title or THEN clause to reflect `id DESC` as the
stable tiebreaker and ensure references to `sess-a`/`sess-b` align with that
rule.
| | Widen only the `/session` service seam | ✅ Yes | Implemented exactly via `handle_session(&CommandContext, raw_args)`. | | ||
| | Dedicated read-only session-list query | ✅ Yes | New `list_session_rows_for_scope(...)` contract added instead of reusing `/resume` listing. | | ||
| | Lifecycle derivation aligned with existing semantics | ✅ Yes | SQLite derives `suspended` only from `session_state.lifecycle_state = 'suspended'`; otherwise `active`; ended rows excluded. | | ||
| | Resumable derived from authoritative current capability | ✅ Yes | Query requires suspended lifecycle, latest compact snapshot reference, existing snapshot row, and `is_resume_capable = 1`. | |
Contributor
There was a problem hiding this comment.
Fix the awkward lifecycle wording.
“requires suspended lifecycle” reads incorrectly; use “requires the lifecycle to be suspended” or similar.
📝 Proposed wording fix
-| Resumable derived from authoritative current capability | ✅ Yes | Query requires suspended lifecycle, latest compact snapshot reference, existing snapshot row, and `is_resume_capable = 1`. |
+| Resumable derived from authoritative current capability | ✅ Yes | Query requires the lifecycle to be suspended, the latest compact snapshot reference, an existing snapshot row, and `is_resume_capable = 1`. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | Resumable derived from authoritative current capability | ✅ Yes | Query requires suspended lifecycle, latest compact snapshot reference, existing snapshot row, and `is_resume_capable = 1`. | | |
| | Resumable derived from authoritative current capability | ✅ Yes | Query requires the lifecycle to be suspended, the latest compact snapshot reference, an existing snapshot row, and `is_resume_capable = 1`. | |
🧰 Tools
🪛 LanguageTool
[style] ~101-~101: The double modal “requires suspended” is nonstandard (only accepted in certain dialects). Consider “to be suspended”.
Context: ...ent capability | ✅ Yes | Query requires suspended lifecycle, latest compact snapshot refe...
(NEEDS_FIXED)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/archive/2026-04-19-slash-session-list/verify-report.md` at
line 101, In the table row starting with "Resumable derived from authoritative
current capability" update the awkward phrase "requires suspended lifecycle" to
clearer wording such as "requires the lifecycle to be suspended" (or "requires
that the lifecycle be suspended") so the third-cell description reads: "Query
requires the lifecycle to be suspended, latest compact snapshot reference,
existing snapshot row, and `is_resume_capable = 1`."
Comment on lines
+384
to
+390
| #### Scenario: Stable tiebreaker preserves repeated ordering for equal activity timestamps | ||
|
|
||
| - GIVEN the current caller scope is authorized to view sessions `sess-a` and `sess-b` | ||
| - AND both sessions have the same authoritative `last_activity` value | ||
| - WHEN the user runs `/session list` multiple times without any underlying session changes | ||
| - THEN the system MUST return `sess-a` and `sess-b` in the same relative order on each execution | ||
| - AND that ordering MUST be produced by a stable secondary ordering rule. |
Contributor
There was a problem hiding this comment.
Fix the equal-timestamp ordering example.
If the implementation’s stable secondary rule is id DESC, sess-b should come before sess-a. Please align this scenario with the implemented tie-breaker.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openspec/specs/sessions/spec.md` around lines 384 - 390, Update the scenario
"Stable tiebreaker preserves repeated ordering for equal activity timestamps" to
match the implemented stable secondary rule `id DESC`: change any example
ordering that shows `sess-a` before `sess-b` so that `sess-b` is listed before
`sess-a` in the GIVEN and THEN lines, and explicitly mention the secondary
ordering rule `id DESC` in the scenario text (referencing the scenario header
and the session ids `sess-a` and `sess-b` so the example aligns with the
implemented tie-breaker).
- sqlite.rs: constrain JOIN to owning session and compact snapshots - service.rs/types.rs: redact payload in inspect, add preview helper - service.rs: load lightweight model for status (no snapshot resolve) - OpenSpec docs: update handle_session contract, add suspended/no-snapshot rule - OpenSpec docs: fix registry example, tiebreaker examples, awkward phrasing Closes #602
This was referenced Apr 21, 2026
Merged
Closed
Merged
This was referenced Apr 29, 2026
8 tasks
8 tasks
This was referenced May 5, 2026
Merged
8 tasks
This was referenced May 10, 2026
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.
Related Issues
Related to Track 2 slash command parity work.
Summary
This PR extends the
/sessionslash-command family with the next browser-oriented slices while preserving the existing family shape.It adds:
/session inspectfor richer current-session inspection/session listfor caller-scoped session browsing ordered by latest activityThe implementation keeps
/sessionas the family hub, preserves/session statusas the compact view, and keeps all forms routed through the shared pre-execution seam. It also updates the runtime memory contract and SQLite implementation to support caller-scoped minimal session rows for slash listing, and archives the corresponding OpenSpec changes.Tested Information
Verified with targeted and hook-enforced checks:
cargo test --manifest-path clients/agent-runtime/Cargo.toml --lib session_commands::service::tests::session_inspect_returns_richer_current_session_view_when_authoritative_data_is_complete -- --exactcargo test --manifest-path clients/agent-runtime/Cargo.toml --lib session_commands::service::tests::session_list_returns_caller_scoped_rows_in_desc_order_with_balanced_output -- --exactcargo test --manifest-path clients/agent-runtime/Cargo.toml --lib pre_execution::tests::ingress_classifies_session_list_through_shared_seam -- --exactcargo test --manifest-path clients/agent-runtime/Cargo.toml --lib memory::sqlite::tests::list_session_rows_for_scope_uses_last_activity_then_id_desc_ordering -- --exact3654 passed, 0 failedReviewer focus:
/session inspectand/session listbehavior inclients/agent-runtime/src/session_commands/clients/agent-runtime/src/memory/sqlite.rs/sessionrouting inclients/agent-runtime/src/pre_execution/mod.rsandsrc/session_commands/registry.rsDocumentation Impact
openspec/specs/slash-command-registry/spec.mdopenspec/specs/sessions/spec.mdopenspec/changes/archive/2026-04-19-slash-session-inspect/openspec/changes/archive/2026-04-19-slash-session-list/Breaking Changes
None intended.
Checklist