fix(project-slug): resolve relative root before deriving slug#38
fix(project-slug): resolve relative root before deriving slug#38Pawel-N-pl wants to merge 1 commit into
Conversation
|
Warning Review limit reached
More reviews will be available in 22 minutes and 44 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
🤖 Augment PR SummarySummary: Fixes Changes:
Notes: Reduces collision risk for manual/relative invocations and aligns slug/hash behavior. 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| # doesn't collapse to the generic "project" fallback. Mirrors get_project_hash(). | ||
| root = Path(project_root or get_project_root()).resolve() | ||
| value = re.sub(r"[^a-z0-9]", "", root.name.lower())[:8] | ||
| return value or "project" |
There was a problem hiding this comment.
Path.resolve() will also collapse symlinks, so callers passing a symlinked project_root may see get_project_slug() change compared to the prior Path(...).name behavior. Is that acceptable given this slug is used in tmux session naming/filtering?
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
There was a problem hiding this comment.
Intentional, and consistent with the sibling get_project_hash, which already calls .resolve(). This makes a project's slug and hash agree on identity instead of diverging (the previous slug used the unresolved name while the hash resolved). For tmux session naming/filtering it's preferable: the same real project reached via different symlinked paths now maps to one consistent slug+hash, rather than fragmenting sessions per symlink. Net effect is removing a pre-existing slug/hash inconsistency, not introducing new behavior.
|
@coderabbitai review |
✅ Action performedReview finished.
|
bma-d
left a comment
bma-d
left a comment
There was a problem hiding this comment.
Thanks for the work here. I'm requesting changes because the slug fix changes the public session prefix for projects opened through a symlinked root.
| root = Path(project_root or get_project_root()) | ||
| # Resolve before .name so a relative root like "." (whose Path(".").name is "") | ||
| # doesn't collapse to the generic "project" fallback. Mirrors get_project_hash(). | ||
| root = Path(project_root or get_project_root()).resolve() |
There was a problem hiding this comment.
This currently resolves the project root before deriving the slug. That fixes . but also makes an absolute symlinked root use the target directory basename, so PROJECT_ROOT=/workspace/linked-root pointing at /workspace/actual-root produces actualro instead of linkedro. Existing sessions named under the symlink prefix are then hidden by project-only filtering, which matches sa-{project_slug}-.
Suggested fix: handle . without changing absolute symlink identity, or document and test that project identity is based on the resolved real path.
fix(story-automator): address maintainer review on bmad-code-org#38/bmad-code-org#39/bmad-code-org#41/bmad-code-org#42
3 participants
What
get_project_slug(".")(or any relative root) returned the generic"project"slug instead of the real directory name.
Why
Path(".").name == "", sore.sub(...)[:8]was empty and fell back to thecollision-prone
"project". Production passes an absolute{project-root}(→ e.g.
upmonaut) so it was unaffected; manual/relative invocations hit it.Fix
.resolve()the root before taking.name, mirroringget_project_hash()inthe same module.
Tests
tests/test_project_slug.py— absolute root, relative"."(regression), andthe non-alphanumeric
"project"fallback.Addresses #35.