Skip to content

refactor: extract session artifact paths - #3186

Merged
thymikee merged 2 commits into
mainfrom
refactor/session-artifact-paths-main
Oct 4, 2026
Merged

thymikee merged 2 commits into
mainfrom
refactor/session-artifact-paths-main

Conversation

@thymikee

@thymikee thymikee commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Summary

Extract ordinary functions for session directories and app-log paths, so device-claim recovery no longer constructs a SessionStore just to locate artifacts. Replace SessionStore.expandHome callers with the existing path function. The home-expansion control now checks the exact result.

Targets main independently. Consolidates #3134 and owning review corrections; 10 files. Part of #3116.

Validation

Commit 1dc0c5eba6dbf8217148e29c49f8cb316a25047a: pnpm check:affected --base d396f3b509 --run passed all runnable checks. 2679 related tests passed. GitHub checks are running on this head.

The initial consolidation (before later keeper fixes) reproduced the pre-consolidation tree at f71996e197 (tree e26fb2928d4a33b05a6092643ff83d9b22d1353f). Previous native evidence and review findings keep their original head attribution.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 10 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread packages/host-kit/src/session-paths.test.ts Outdated
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.95 MB 4.95 MB -349 B
Package (unpacked) 4.95 MB 4.95 MB -349 B
Package (download) 1.48 MB 1.48 MB +278 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.2 ms 27.8 ms -0.4 ms
CLI --help 84.4 ms 84.9 ms +0.5 ms

@thymikee thymikee changed the title refactor: resolve session artifact paths without a store refactor: extract session artifact paths Oct 3, 2026
@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

The PR is ready at 1dc0c5e. Not blocking: the doc comment on isSafeSessionSegment in packages/host-kit/src/session-paths.ts still names SessionStore.resolveSessionDir, but the owner is now resolveSessionDir in src/daemon/session-artifact-paths.ts, and resolveSessionAppLogPath and resolveSessionAppLogPidPath have no direct test, so take or leave both. All 19 checks pass. I did not run the test suite or the affected-package check myself, and I did not do a live device run, since this only moves path resolution and the app-log paths resolve to the same values as before. I know of no conflicts. Nothing else stands in the way of merge, and Cubic has not reviewed this head yet.

The one earlier thread does not apply, so please resolve it: Cubic's P3 on the session-paths test is fixed at this head, which now asserts exact equality with the joined home path (#3186 (comment)).

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Oct 3, 2026
@thymikee
thymikee merged commit 83cabf2 into main Oct 4, 2026
19 checks passed
@thymikee
thymikee deleted the refactor/session-artifact-paths-main branch October 4, 2026 01:35
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-10-04 01:35 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant