Repository navigation
test(session-store): ignore the checkout path in the secret-word check - #197
Conversation
"terminal window descriptors persist atomically without scrollback or environment" saves a descriptor whose cwd is process.cwd() and then checks the whole file against /buffer|environment|capability|token/. Run from a checkout whose path contains one of those words (for example a worktree named after an environments feature), the cwd alone matched and the test failed on every run. The check now leaves the descriptor's own cwd value out; everything else in the file is checked as before.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@coderabbitai full review |
Action performed
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Persistence assertion tests/terminal-session-store.test.mjs |
The assertion excludes the serialized cwd path before checking for buffer, environment, capability, or token. |
Estimated code review effort: 1 (Trivial) | ~2 minutes
Change: Other
Suggested reviewers: howdeploy
Merge Risk: 
e8b4c
-
This test-only change removes checkout-path false failures without changing application behavior.
Architecture risk: 
e8b4c
-
The change affects 1 system.
Changed systems:
testsArchitecture concerns
No architecture-level concerns identified.Review details
-
Systems and components
- observed — tests (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
- observed — Modified behavior in tests/terminal-session-store.test.mjs: The persistence assertion now excludes the serialized
cwdpath before checking for forbidden terms, so a matching word in the checkout path does not affect the test.
-
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the test-only change: excluding the checkout path from the secret-word check in the session store test. Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Split out of #159 and rebased on current main (2367505): fix 12 of #159, unchanged in substance. Small and independent; it does not depend on the other parts of #159. Fix 11 of #159 (the engine-tabs click race) is not included: main already fixes it in 6e36e98 (
ready(tabId, afterRevision)).Problem
"terminal window descriptors persist atomically without scrollback or environment" in
tests/terminal-session-store.test.mjsfails on every run when the checkout path contains "environment", "token", "buffer" or "capability" (for example~/environment/CanvasTTY, or a worktree named after an environments feature).Cause
The saved descriptor's
cwdisprocess.cwd(), and the test scanned the whole saved file,cwdincluded, against/buffer|environment|capability|token/. The checkout path alone matched.Fix
The scan leaves the descriptor's own
cwdvalue out (it is replaced by""before matching). Everything else in the file is checked as before, so a leaked buffer, environment, capability or token field still fails the test. One test file changes (+2/-1); no product code changes.Test
environment-check: on 2367505 without the fix, 1 of 7 tests fails (this one,doesNotMatch); with the fix, 7 of 7 pass. From a normal checkout path both pass.Verification
npm run build(includes typecheck) andnpm run audit:secrets: pass.Summary by CodeRabbit