refactor: split session.rs into directory module (FM-1, FM-2, FM-3) #63
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "chore/split-session-tools"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
crates/cognix-mcp/src/tools/session.rs(752 lines) intosession/{mod,create,get,list}.rs. Public API preserved via thin facademod.rs(pub use).notes/cognix-evaluation-guide.mdsection 3.3 +test_write_tool_rejects_parent_traversal_outside_workspaceintegration test.tests/integration/q02_tool_matrix.rsfrom section 5 of evaluation guide.10a722f): tighten filesystem path normalization (WindowsPrefixparticipates in workspace check, parent-traversal rejection in relative components) and surface previously-swallowed session storage failures via the responsewarningsfield.Fail-open visibility changes (
10a722f)The hardening commit makes three observable changes to session-tool responses. Each strengthens fail-open observability per
.claude/rules/patterns.md(DegradedResult<T>); none change happy-path output.handle_create_sessionsession persistence degraded: …to the responsewarningsarray; ID still returnedhandle_get_sessionhandle_list_sessionswarningsfield describing the degraded read; list field still presentAll three behaviors are covered by tests in
session/{create,get,list}.rs.FM-DG-01 layout decision
Directory module:
session/{mod,create,get,list}.rs.internal.rsnot created — no shared implementation items qualified;mod.rsis 9 physical lines (well under the 100-line threshold).make_storage(3-line test fixture) duplicated into each test module rather than shared.File sizes (post-split)
mod.rsget.rscreate.rslist.rsAll under the 400-line target; none exceed the 800-line hard limit.
Test attribution
9 tests to
create.rs, 4 toget.rs, 5 tolist.rs. None spanned multiple handlers.Public API gate (fallback)
All 6 pre-split public symbols re-exported from
session/mod.rsviapub use:create_session_definition,handle_create_session,get_session_definition,handle_get_session,list_sessions_definition,handle_list_sessions.cognix-serverbuilds unchanged.Platform coverage note
The new
Prefixarm innormalize_absolute_componentsis exercised bytest_normalize_absolute_components_preserves_windows_prefixandtest_normalize_workspace_relative_path_allows_valid_windows_workspace_path, both#[cfg(windows)]. Linux CI cannot reach the branch becausePath::components()never yieldsPrefixoutside Windows; the arm is documented inline atcrates/cognix-server/src/registration/filesystem.rs.Test plan
cargo fmt --checkcargo clippy --workspace --all-targets -- -D warningscargo test --workspace(2743 passed, 9 ignored)cargo build --release --workspacecargo docwith strict RUSTDOCFLAGStest_write_tool_rejects_parent_traversal_outside_workspacepassesSelf-Review Checklist
unwrap()in library codeprintln!/dbg!/eprintln!FM-1: Split crates/cognix-mcp/src/tools/session.rs (752 lines) into session/{mod,create,get,list}.rs to stay below 800-line ceiling and unblock the planned file-length CI gate. Public API preserved via thin facade `mod.rs` (pub use). FM-2: add S-20 path-traversal criterion + parent-traversal test. FM-3: cross-reference q02_tool_matrix.rs from §5 of evaluation guide. File sizes: create.rs 263 lines get.rs 170 lines list.rs 307 lines mod.rs 9 lines (thin facade)L2: document Windows-only Prefix arm in normalize_absolute_components (Path::components never yields Prefix on Unix, so coverage lives in the cfg(windows) tests). L3: drop unused clippy::panic allow from file_tools_mcp.rs to match the existing integration-test convention (unwrap_used only).