Skip to content

fix(session): keep published session paths machine-independent - #1591

Open
huong8373tt-beep wants to merge 1 commit into
Tencent:mainfrom
huong8373tt-beep:teamai-cli-1579
Open

huong8373tt-beep wants to merge 1 commit into
Tencent:mainfrom
huong8373tt-beep:teamai-cli-1579

Conversation

@huong8373tt-beep

@huong8373tt-beep huong8373tt-beep commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • store published session cwd relative to the git workspace root or configured non-git project root;
  • preserve monorepo subdirectories while removing absolute machine paths from meta.json and _index.json;
  • keep placeholder/non-bound paths empty instead of guessing a local path;
  • keep the archive key and native session lookup behavior unchanged;
  • avoid rewriting an unchanged index during rebuild, so a no-op pull leaves the checkout clean;
  • handle non-git workspace children whose names begin with .. without rejecting valid descendants;
  • compare canonical paths so sessions pushed through symlinked workspace paths retain the correct relative location.

Part of #1579 (Slice 1: relative archived cwd).

Validation

  • npx tsc --noEmit — passed;
  • git diff --check — passed;
  • focused archive-path regression coverage includes git roots, monorepo children, non-git children, ..cache names, and symlinked paths.
  • Full Vitest execution is currently unavailable locally because the optional Rolldown native binding is not installed; the repository's official CI will provide the complete test run.

Scope and compatibility

  • Existing archives remain readable.
  • Native cwd is still used for local lookup and archive identity before publication.
  • Only published metadata/index projections change.
  • No credentials or external service calls are required for the fixture-based verification.

Disclosure and authorization

Slai Agent Runtime performed the investigation, implementation, and test execution. The account owner authorized this contribution and is responsible for its content.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Missing real-CLI verification — PR description, Validation. This changes runtime behavior, but the record lists only tests, typechecking, linting, and a build. The first Code Review Rules item explicitly requires an end-to-end / real-CLI record. Add one representative CLI run exercising publication and checking the archived metadata/index; no full provider × agent matrix is required.

  • [P2 non-blocking] Unrelated Windows hook changes — src/hooks.ts:1655. The new cmd.exe wrapper, associated hook tests, and dispatcher documentation address [bug] Codex 0.166.0 on Windows: cmd-style hooks emit BackgroundJob output when executed through PowerShell #1578, not the stated session-path fix. This violates the surgical-changes rule. Move these changes into a separate PR.

  • [P3 nit] Valid directories beginning with .. lose their path — src/session-flow/sync.ts:68. For a workspace /repo and session cwd /repo/..cache, the relative path is ..cache, but this guard publishes '' even though it remains inside the workspace. Reject only .. itself or paths beginning with ..${path.sep}.

Review was diff-only; no PR code was run, built, or installed.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This was generated by AI during triage.

Thanks for picking up Slice 1 of #1579. I ran the built CLI at 7c6beb6 and at main with one Claude Code session per row of the Slice 1 table (record below). The root, monorepo, workspace child, no-repo and removed-directory rows are right, session migrate --push stores packages/api, a session with a relative cwd resumes in another directory with its turns, and an older CLI and this one each read the other's archive (session list, pull, resume), so no breaking-change handling is needed. The no-op index check also keeps a CRLF checkout clean, where main rewrites _index.json.

To fix before merge:

  • [P1 blocking] A plain folder inside a non-git workspace archives as "", not backend. archivedCwd calls detectProjectConfig(cwd), whose non-git branch reads only <cwd>/.teamai, so ws/backend never finds ws/.teamai. Walk up from the cwd to the workspace root, and add a test for this row.
  • [P1 blocking] A cwd spelled through a symlink loses its path, and macOS CI is red on it. The workspace root comes back as a real path (sync.ts:61-64) and is compared with the cwd as given (:66), so /tmp/... on macOS gives ../../… and then "". macOS Lint & Test (Node 22 and 24) fail at session-sync.test.ts:111 (expected '' to be '.') and :143. Realpath the cwd before path.relative, falling back when it no longer exists.
  • [P1 blocking] The hook commit breaks CI and is already on main. 87a8d1c5 is the same patch as #1585, merged as 0a8230db, but without its fixture update, so hooks-skill-retire.test.ts:75 fails on Windows, Ubuntu and macOS. Rebasing onto main and dropping it leaves this PR with no hook changes.
  • [P2 non-blocking] The CI review's ..cache point reproduces: a session in app/..cache archives as "". relative === '..' || relative.startsWith('..' + path.sep) fixes it.
  • [P2 non-blocking] Closes #1579 would close the issue with slices 2-4 undone. I'd use Part of #1579.
  • [P3 nit] writeArchive and saveSessionLocked became async with nothing to await (the latter's comment says it stays synchronous); and the index no-op check could live only in writeIndex, comparing parsed entries.

Real-CLI record:

Built at 7c6beb65 (PR) and 64b031ed (baseline). Isolated HOME, empty GIT_CONFIG_GLOBAL, TEAMAI_NONINTERACTIVE=1.
teamai session push --source claude-code --session <id> --scrub -y --cwd <dir>   (exit 0 for every row)

archived cwd (meta.json and _index.json)   PR build         baseline
app (git root)                             "."              absolute
app/packages/api                           "packages/api"   absolute
app/..cache                                ""               absolute
ws/svc-a (child repo)                      "."              absolute
ws (non-git workspace root)                "."              absolute
ws/backend (plain folder)                  ""               absolute
scratch (no repo, no config)               ""               absolute
removed directory                          ""               absolute

teamai session migrate m-mono -s claude-code -t claude-code --push --scrub -y   exit 0, origin.cwd="packages/api"
teamai session resume claude-code_monorepo-subdir_20261027 --platform claude-code --cwd <other>   exit 0, Messages: 4
Baseline reads the PR's archive: session list, session pull (9 sessions), session resume: exit 0.
PR reads the baseline's archive: session list (18 sessions), session resume: exit 0.

@huong8373tt-beep

Copy link
Copy Markdown
Contributor Author

I reviewed the requested changes and prepared a focused follow-up locally. I will remove the unrelated Windows hook commit from this branch, fix the non-git child and symlink path cases, correct the ..cache containment check, retain the real-CLI verification record, and change the issue reference to Part of #1579 so later slices remain open.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Missing real-CLI verification — PR description, Validation. The record still lists only tests, typechecking, linting, and a build. Runtime changes require an end-to-end / real-CLI record under the first Code Review Rules item. Document one representative CLI run exercising publication and checking archived metadata/index; no full provider × agent matrix is required.

  • [P2 non-blocking] Unrelated Windows hook changes — src/hooks.ts:1655. The cmd.exe wrapper, associated hook tests, and dispatcher comments address [bug] Codex 0.166.0 on Windows: cmd-style hooks emit BackgroundJob output when executed through PowerShell #1578 rather than the stated session-path fix. These remain unrelated changes under the surgical-changes rule; move them into a separate PR.

Resolved: The earlier ..cache finding is fixed: the guard now rejects only .. and parent-directory prefixes, with regression coverage for valid two-dot child names.

Review was diff-only; no PR code was run, built, or installed.

@github-actions

Copy link
Copy Markdown
  • [P1 blocking] Missing real-CLI verification — PR description, Validation. This changes runtime behavior, but Validation records only tests, typechecking, linting, a build, and a diff check. The first ## Code Review Rules item explicitly requires an end-to-end / real-CLI record. Document one representative CLI run exercising publication and checking archived metadata/index; no full provider × agent matrix is required.

Resolved: The unrelated Windows hook changes are absent from the current diff. The ..cache guard is fixed and has regression coverage.

No additional confident findings in the specified diff. Review was diff-only; no PR code was run, built, or installed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants