Repository navigation
Conversation
|
@codex review |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe changes distinguish displaced psmux registries derived from another project’s path and detect superseded control sessions in the project registry. Attach and cleanup report matching sessions. Cleanup can include session termination commands in text output and JSON. Changespsmux Registry and Session Reporting
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant cmd_attach
participant superseded_ctl_sessions
participant PsmuxMultiplexer
participant cmd_cleanup
participant cleanup_document
cmd_attach->>superseded_ctl_sessions: find superseded session names
superseded_ctl_sessions->>PsmuxMultiplexer: list sessions
PsmuxMultiplexer-->>superseded_ctl_sessions: session names
superseded_ctl_sessions-->>cmd_attach: matching session names
cmd_cleanup->>cleanup_document: include legacy_kill_commands
Suggested reviewers: Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 69.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 5 files. (2 skipped: … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies the primary change: cleanup now reports pre-upgrade bmad-loop-ctl control sessions. 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
Docstring coverage is 69.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 5 files. (2 skipped: 1 unsupported, 1 too large.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit checks the registry rows,
Finds an old control session that glows.
“Here’s how to attach,” the notices say,
“And here’s the command to clear the way.”
Across project roots, names now show,
The rabbit hops on, pleased to know.
Comment @coderabbitai help to get the list of available commands.
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfed3f12ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ved root A displaced PSMUX_DATA_DIR shaped like a root bmad-loop derived for another project path (or for this project before a rename) holds sessions whose owner cannot be proven from here: the tag hashes a path and run ids are caller-supplied. Report its sessions in full as before, but name the registry for what it is instead of calling them "not migrated", in the cleanup stderr line, the TUI toast and the displaced-root note. The --json document is unchanged. Closes bmad-code-org#906
…sion After the control session went back to one fixed name, a session started under the old bmad-loop-ctl-<16 hex> name could keep running in the project's own registry, holding a parked run window, and nothing named it: cleanup said nothing to clean up, --json reported no leftovers and attach found nothing. cleanup now names it on stderr and in sessions.legacy_leftovers, with the command that ends it in sessions.legacy_kill_commands, and attach names it when it finds nothing to attach. attach does not adopt the session: stop, the parked-window sweep and the decision return path do not address it. Refs bmad-code-org#733
cfed3f1 to
b18bc67
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b18bc67917
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…the registry The previous release minted the old control-session name from the resolved, case-folded registry root, so an honoured PSMUX_DATA_DIR that is a symlink, junction or other spelling of the project's derived registry held that name. Compare the registry identities the same way instead of as spelled. A root that cannot be resolved falls back to the as-spelled comparison and reports the degrade through on_fault, which cleanup lists under legacy_unverified and attach prints as a note. Refs bmad-code-org#733
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ddb8e8a29
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…try root With [mux] honor_ambient_psmux_data_dir keeping an operator's own root, the previous release salted the control-session digest with that root, so the session there carries a name the derived registry's never matched. Derive the name for the registry in force with that release's formula, salt included, and match it there: either digest covers this project's derived root, so the session is this project's wherever it stands. A root that cannot be derived or resolved is no longer an empty answer: the fault goes to on_fault, which cleanup lists under legacy_unverified and attach prints as a note. Refs bmad-code-org#733
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 170c36e9cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…smux The recovery commands for a superseded control session were always spelled as psmux PowerShell lines, so an out-of-tree registry-namespaced backend got a command that fails there or reaches the wrong registry. Print the paste-ready command only when the selected backend is psmux; on any other backend name the session without one, in the cleanup text, as a null in sessions.legacy_kill_commands, and in the attach note. Refs bmad-code-org#733
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bed31eef9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Review status: rounds 1–4 of the automated review are fixed on this branch, each with a test, and CI is green on
Neither is reachable with the run ids and backends bmad-loop itself produces. Both threads are answered and link #915. This PR is stacked on #911, so please merge #911 first. |
1 participant
Stacked on #911 (#906). Merge #911 first; until then this diff also shows its commits.
After #900 the psmux control session went back to one fixed name,
bmad-loop-ctl. A control session started under the oldbmad-loop-ctl-<16 hex>name can keep running in the project's own registry and hold a parked run window. Nothing named it (reported on #733):cleanup --dry-runprintednothing to clean up;cleanup --jsonreported"legacy_leftovers": [];attach <run>answerednothing to attach.An operator who had not read the CHANGELOG had no trace of it.
Change
cleanupnames a pre-upgradebmad-loop-ctl-<16 hex>session in the project's own registry:left in <registry> (not migrated): …, followed by the command that ends it;--jsonundersessions.legacy_leftovers, with the command in the newsessions.legacy_kill_commands. The field is additive; existing fields are unchanged.attach, when it finds nothing to attach, names the session and prints the command that attaches to it.stop, the parked-window sweep and the decision return path do not address it, so landing the operator there would half-adopt it. The reasoning is in_note_superseded_ctl_for_attach.multiplexer-backends.md,reference/cli.md) and the CHANGELOG are updated.Verification
Live, on psmux 3.4.0 from PowerShell 7 with
SHELLunset, using the repro from the #733 comment. The legacy window was tagged throughmux.set_window_option(wid, runs.PROJECT_OPTION, runs.project_tag(project)).cleanupnames the session on stderr.--jsonlists it inlegacy_leftoversand maps it inlegacy_kill_commands.attachnames it.psmux lsthen reports no server.Tests:
tests/test_runs.pyandtests/test_cli.pycover the report, the JSON field and the attach note.WinError 1314and fail the same way on main.test_runs+test_cli1566 passed.test_tui_app400 passed.Refs #733
Summary by CodeRabbit
Bug Fixes
Documentation