Finalproject nmc - #258
Open
nestormata wants to merge 3 commits into
Open
Conversation
📝 WalkthroughWalkthroughThe PR replaces project templates with detailed Project Vault documentation, records the BMad development prompt history, and adds ChangesProject Vault documentation
BMad workflow history
Story planning and delivery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant plan-story
participant Worktree
participant StoryAgent
participant ReviewAgent
participant sprint-status.yaml
User->>plan-story: select backlog story
plan-story->>Worktree: create or resume plan branch
plan-story->>StoryAgent: generate story artifact
StoryAgent-->>plan-story: confirm story file
plan-story->>ReviewAgent: review story independently
ReviewAgent-->>plan-story: return findings artifact
plan-story->>sprint-status.yaml: set story to ready-for-dev
plan-story->>Worktree: commit planning artifacts
plan-story-->>User: report branch, commit, and review findings
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@readme.md`:
- Around line 509-515: The schema’s API-key expiration model is inconsistent
with the requirement for independently expiring keys. Add a nullable expires_at
timestamp to MACHINE_USER_KEY, alongside revoked_at, so each key can be renewed
and expire independently; keep the existing machine-user fields unchanged.
- Around line 1094-1107: Update the integration test for queries without
app.current_org_id to assert that the database returns an error, not zero rows.
Keep the tenant-isolated query assertions unchanged and ensure the test verifies
missing tenant context cannot be silently treated as an empty result.
- Around line 783-785: Update the README’s project-role documentation and
permission contract to explicitly define the Audit role, including its enum
representation and access enforcement for security_audit_log entries, or remove
Audit from the documented API behavior and related permission descriptions if it
is not supported.
- Around line 362-364: Actualiza la documentación de “Aislamiento de
organización” en las secciones señaladas para que el contrato de prevención de
bypass de RLS sea exacto: documenta el uso de FORCE ROW LEVEL SECURITY junto con
la separación entre propietario de tablas y rol de runtime, o limita
explícitamente la afirmación al modelo real de ownership; mantén coherencia
entre ambas secciones.
- Around line 975-985: Move the pg_try_advisory_xact_lock acquisition into the
single database transaction that creates the SECRET_VERSION, ROTATION_EVENT,
ROTATION_CHECKLIST_ITEM records, SECURITY_AUDIT_LOG entry, and notification
queue record. Ensure the lock is checked after BEGIN and return 409 when
unavailable, so the transaction-scoped lock protects all rotation writes.
- Around line 401-413: The audit atomicity test should fail at the audit
insertion itself rather than relying on a separate SET LOCAL statement_timeout
call. Update the revealSecret test to mock or otherwise force the
security_audit_log insert to reject during the operation, assert that
revealSecret rejects, and verify both the secret operation and access log remain
rolled back.
- Around line 147-155: Align the README quick-start, project tree, production
example, and service URL sections to use the same Docker Compose file path and
consistent host-to-container port mappings. Update the `docker compose up
--build` command and Web/API/Health URLs to match the ports exposed by the
documented compose configuration, preserving the existing service names and
commands where applicable.
- Around line 944-948: Update the offline secret fallback requirement in the
pipeline scenario to define a maximum cache age/TTL, invalidate cached values
when secrets are revoked or rotated, and fail closed when the cached value is
expired or invalidated. Preserve the existing three-failure trigger, audit
logging, and administrator alert behavior for valid cached secrets.
In `@skills/my-epic-retro/SKILL.md`:
- Around line 89-90: Define implementation_artifacts before Step 1, using the
concrete _bmad-output/implementation-artifacts path from Quick Reference, and
reuse that binding consistently for prior-retro discovery and retro saving in
the affected steps.
- Around line 218-254: Update the post-retrospective flow around the routing
options and sprint-status instructions to explicitly reopen, edit, and save the
retro document after Step 3, recording repeat-finding overrides verbatim and
tooling/skill outcomes in the specified sections or tables. Make the
sprint-status last_updated update and commit conditional on at least one new
backlog entry; when none are created, do not perform a misleading no-op commit.
- Around line 190-203: Clarify the write order between the ceremony’s status
update and the confirmation gate in Step 11: explicitly state whether marking
epic-{{epic_number}}-retrospective as done is allowed before confirmation as an
exception, or require deferring both the retrospective file and
sprint-status.yaml writes until confirmation. Ensure the instructions
consistently preserve the selected ordering.
In `@skills/pick-story/SKILL.md`:
- Around line 277-285: Update the C1 commit instructions to use git add -A
instead of interactive git add -p, ensuring new implementation and test files
are staged non-interactively. Retain the requirement to inspect the staged diff
before running the existing commit command.
- Around line 367-375: Update the worktree tracking in the Path C review flow so
C7 recognizes sessions entered through either C0 or B2. Call ExitWorktree with
action "keep" whenever either path attached the session, while preserving the
no-op behavior for sessions that entered neither worktree path.
- Around line 242-258: Update the pick-story routing in Step 1 and the B5
“Continue later” flow so a fresh session can detect and resume the existing
feature branch after /bmad-dev-story completes, even when main’s
sprint-status.yaml still says ready-for-dev. Persist the review state where Step
1 reads it or independently discover the existing feature branch, and route that
case directly to Path C without recreating the branch.
In `@skills/plan-story/SKILL.md`:
- Around line 116-129: Before each EnterWorktree call in the resume and
orphaned-worktree flows, run git status --porcelain for the target worktree and
halt if it reports uncommitted changes or other conflicts; only attach when
clean. Apply this to skills/plan-story/SKILL.md lines 116-129 and
skills/pick-story/SKILL.md lines 203-210, preserving the existing reattachment
and existing-file checks.
- Around line 140-142: Update the worktree naming flow in the plan story
instructions to compute one shortened identifier when the 64-character limit is
exceeded, preserving the required prefix and truncating only the descriptive
slug. Use that identifier consistently for branch creation, worktree paths,
resume lookup, commit handoff, cleanup, and Step 7 reporting instead of mixing
shortened and full names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a534ee2-fd2f-44ec-ab30-9a541d1f5342
📒 Files selected for processing (5)
prompts.mdreadme.mdskills/my-epic-retro/SKILL.mdskills/pick-story/SKILL.mdskills/plan-story/SKILL.md
Comment on lines
+147
to
+155
| ```bash | ||
| cp .env.example .env # configurar variables de entorno | ||
| docker compose up --build # iniciar todos los servicios | ||
| ``` | ||
|
|
||
| Servicios disponibles en: | ||
| - Web: http://localhost:5173 | ||
| - API: http://localhost:3000 | ||
| - Health: http://localhost:3000/health |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the README and Compose-related references, then inspect the relevant sections.
git ls-files | rg '(^|/)(readme\.md|README\.md|docker/|compose\.ya?ml|docker-compose\.ya?ml)$' || true
printf '\n--- README references ---\n'
rg -n "docker compose|docker/|5173|3000|3001|health|compose" readme.md README.md docker-compose.yml docker-compose.yaml docker/ -S || true
printf '\n--- README excerpt around cited lines ---\n'
sed -n '135,205p' readme.md
printf '\n--- README excerpt around architecture section likely cited ---\n'
sed -n '295,325p' readme.md
printf '\n--- README excerpt around later cited section ---\n'
sed -n '1140,1165p' readme.mdRepository: LIDR-academy/AI4Devs-finalproject
Length of output: 7198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find compose/port declarations in repository files to compare doc claims against config.
for f in $(git ls-files | rg '(^|/)(compose\.ya?ml|docker-compose\.ya?ml|Dockerfile|readme\.md|README\.md)$'); do
printf '\n### %s ###\n' "$f"
rg -n "ports:|5173|3000|3001|health|localhost|expose:|docker compose|compose" "$f" -S || true
doneRepository: LIDR-academy/AI4Devs-finalproject
Length of output: 2228
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If a compose file exists under docker/, inspect it directly for service ports and volumes.
fd -a -t f 'compose*.yml' . docker . 2>/dev/null || true
fd -a -t f 'docker-compose*.yml' . docker . 2>/dev/null || true
for f in docker/compose.yml docker/compose.yaml docker/docker-compose.yml docker/docker-compose.yaml compose.yml compose.yaml docker-compose.yml docker-compose.yaml; do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
cat -n "$f"
fi
doneRepository: LIDR-academy/AI4Devs-finalproject
Length of output: 171
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Narrowly inspect the exact README lines referenced by the review comment.
nl -ba readme.md | sed -n '145,160p'
printf '\n'
nl -ba readme.md | sed -n '186,192p'
printf '\n'
nl -ba readme.md | sed -n '307,316p'
printf '\n'
nl -ba readme.md | sed -n '1150,1160p'Repository: LIDR-academy/AI4Devs-finalproject
Length of output: 211
Align the Docker setup docs. The quick start uses docker compose up --build, but the tree and production example point to docker/docker-compose.yml/docker-compose.yml, and the port docs mix host 5173/3000 with container 3000/3001. Make the compose path and host↔container port mappings consistent across these sections.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.md` around lines 147 - 155, Align the README quick-start, project
tree, production example, and service URL sections to use the same Docker
Compose file path and consistent host-to-container port mappings. Update the
`docker compose up --build` command and Web/API/Health URLs to match the ports
exposed by the documented compose configuration, preserving the existing service
names and commands where applicable.
Comment on lines
+362
to
+364
| **Aislamiento de organización** | ||
| - `org_id` en cada entidad del esquema; aplicado mediante PostgreSQL Row-Level Security (RLS) a nivel de base de datos — una query sin `org_id` falla en la BD, no en la aplicación; no puede ser omitida por bugs de código o queries SQL directas | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map repository files related to RLS, roles, and database setup.
printf '\n== Files mentioning RLS / roles ==\n'
git ls-files | rg -n '(^|/)(readme\.md|README\.md|.*sql$|.*prisma.*|.*drizzle.*|.*migrations?.*|.*schema.*|.*db.*|.*postgres.*|.*security.*)$' || true
printf '\n== Search for RLS / BYPASSRLS / FORCE ROW LEVEL SECURITY / ownership ==\n'
rg -n -i 'row level security|rls|bypassrls|force row level security|row-level security|owner.*table|table owner|role|postgres' . \
--glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' --glob '!**/.git/**' || true
printf '\n== README excerpt around the cited lines ==\n'
if [ -f readme.md ]; then
sed -n '340,390p' readme.md | cat -n
fi
printf '\n== README around later mention (if any) ==\n'
if [ -f readme.md ]; then
sed -n '1065,1100p' readme.md | cat -n
fiRepository: LIDR-academy/AI4Devs-finalproject
Length of output: 14123
Complete the RLS bypass-prevention contract in readme.md:362-364 and 1081-1087. ENABLE ROW LEVEL SECURITY plus “NO BYPASSRLS” does not rule out owner/privileged bypasses; add FORCE ROW LEVEL SECURITY, document the table-owner/runtime-role split, or narrow the claim to the actual ownership model.
🧰 Tools
🪛 LanguageTool
[style] ~363-~363: En sentido figurado es preferible usar otras expresiones.
Context: ...nte PostgreSQL Row-Level Security (RLS) a nivel de base de datos — una query sin org_id ...
(A_NIVEL_DE)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.md` around lines 362 - 364, Actualiza la documentación de “Aislamiento
de organización” en las secciones señaladas para que el contrato de prevención
de bypass de RLS sea exacto: documenta el uso de FORCE ROW LEVEL SECURITY junto
con la separación entre propietario de tablas y rol de runtime, o limita
explícitamente la afirmación al modelo real de ownership; mantén coherencia
entre ambas secciones.
Comment on lines
+401
to
+413
| **Test crítico destacado — invariante de auditoría:** | ||
| ```typescript | ||
| // El log de auditoría se escribe en la misma transacción que la operación | ||
| // Si la escritura de auditoría falla, la operación completa debe fallar | ||
| it('operation fails atomically when audit write fails', async () => { | ||
| // Simula fallo de inserción en security_audit_log | ||
| await db.execute(sql`SET LOCAL statement_timeout = '1ms'`); // fuerza timeout | ||
| await expect(secretsService.revealSecret(secretId, userId)) | ||
| .rejects.toThrow(); | ||
| // Verifica que el secret_version tampoco se registró como accedido | ||
| const accessLog = await auditLog.findBySecret(secretId); | ||
| expect(accessLog).toHaveLength(0); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the referenced file and inspect the nearby section with line numbers.
if [ -f readme.md ]; then
echo "=== readme.md exists ==="
wc -l readme.md
sed -n '380,440p' readme.md | cat -n
else
echo "readme.md not found"
git ls-files | rg -n '(^|/)(readme\.md|README\.md)$'
fi
# Find mentions of the test names / symbols to understand the surrounding implementation.
rg -n "operation fails atomically when audit write fails|revealSecret|security_audit_log|statement_timeout|findBySecret|auditLog" -S .Repository: LIDR-academy/AI4Devs-finalproject
Length of output: 4990
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the implementation and any test/harness code that would show transaction scope.
rg -n "revealSecret\\s*\\(|security_audit_log|audit log|statement_timeout|BEGIN|transaction" -S --glob '!readme.md' .
# If files are found, inspect the most relevant ones with line numbers.Repository: LIDR-academy/AI4Devs-finalproject
Length of output: 307
Inject the audit failure at the audit write itself. SET LOCAL statement_timeout in a separate call won’t reliably trip the later revealSecret transaction; make the audit insert fail directly and assert that the secret operation and access log both roll back.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.md` around lines 401 - 413, The audit atomicity test should fail at
the audit insertion itself rather than relying on a separate SET LOCAL
statement_timeout call. Update the revealSecret test to mock or otherwise force
the security_audit_log insert to reject during the operation, assert that
revealSecret rejects, and verify both the secret operation and access log remain
rolled back.
Comment on lines
+509
to
+515
| MACHINE_USER_KEY { | ||
| uuid id PK | ||
| uuid machine_user_id FK "NOT NULL" | ||
| string key_hash "NOT NULL — BLAKE2b" | ||
| bool is_active "NOT NULL DEFAULT true" | ||
| timestamp created_at "NOT NULL DEFAULT now()" | ||
| timestamp revoked_at "nullable" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Model API-key expiration at the key level.
The requirements promise configurable API-key expiration, but key_expiry belongs to MACHINE_USER; MACHINE_USER_KEY has no expiration field. With multiple active keys, individual renewal and expiry cannot be represented. Add expires_at to MACHINE_USER_KEY or change the requirements to expire the whole machine user.
Also applies to: 950-953
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.md` around lines 509 - 515, The schema’s API-key expiration model is
inconsistent with the requirement for independently expiring keys. Add a
nullable expires_at timestamp to MACHINE_USER_KEY, alongside revoked_at, so each
key can be renewed and expire independently; keep the existing machine-user
fields unchanged.
Comment on lines
+783
to
+785
| Retorna entradas del security_audit_log filtradas. Requiere rol Owner | ||
| o rol Audit explícito. Los Admins solo pueden acceder a entradas de sus proyectos. | ||
| Soporta paginación con cursor. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Define the documented Audit role.
The API permits an explicit Audit role, but the documented project-role enum only contains owner, admin, member, and viewer (Line [466]). Define where Audit is modeled and enforced, or remove it from the permission contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.md` around lines 783 - 785, Update the README’s project-role
documentation and permission contract to explicitly define the Audit role,
including its enum representation and access enforcement for security_audit_log
entries, or remove Audit from the documented API behavior and related permission
descriptions if it is not supported.
Comment on lines
+242
to
+258
| `/bmad-dev-story` updates the story file and flips `sprint-status.yaml` to `review` on completion | ||
| itself — the agent does not need a separate instruction to do that. | ||
| - What to report back (keep it under ~300 words): files changed, a one-line summary per acceptance | ||
| criterion of what was implemented, final test run status, and any blockers or ACs left incomplete. | ||
|
|
||
| Wait for the agent's report. Do not implement inline in this session. | ||
|
|
||
| ### B5: HALT — Implementation Complete | ||
|
|
||
| Relay the agent's report to the user. Note that `/bmad-dev-story` has already moved the story to | ||
| `review` in `sprint-status.yaml`, so the story is safe to pick back up at any time. | ||
|
|
||
| Ask the user how to proceed: | ||
| 1. **Continue now** → go to **Path C** in this same session. | ||
| 2. **Continue later** → end this skill invocation here. Since status is already `review`, a fresh | ||
| `pick-story <story-id>` invocation in a new session will route straight to Path C (via Step 1's | ||
| routing table) with a clean context for review/CI/PR. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Fix status visibility across sessions.
/bmad-dev-story updates sprint-status.yaml inside the feature worktree, but a later invocation starts from main and reads main’s copy. Until the feature branch is merged, that copy still says ready-for-dev, so a fresh session will route to Path B rather than Path C and may try to recreate an existing feature branch. Persist routing state where Step 1 reads it, or discover/resume the existing feature branch independently of main’s status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/pick-story/SKILL.md` around lines 242 - 258, Update the pick-story
routing in Step 1 and the B5 “Continue later” flow so a fresh session can detect
and resume the existing feature branch after /bmad-dev-story completes, even
when main’s sprint-status.yaml still says ready-for-dev. Persist the review
state where Step 1 reads it or independently discover the existing feature
branch, and route that case directly to Path C without recreating the branch.
Comment on lines
+277
to
+285
| ### C1: Commit Implementation | ||
|
|
||
| Stage all changed files and commit: | ||
| ```bash | ||
| git add -p # review staged changes | ||
| git commit -m "feat(<scope>): <short description of story implementation> | ||
|
|
||
| Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>" | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Stage new implementation files non-interactively.
git add -p is interactive and does not reliably include untracked files. If the story creates a new source or test file, C1 can commit only the modified tracked files while silently omitting the new files. Use git add -A, inspect the staged diff, then commit.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/pick-story/SKILL.md` around lines 277 - 285, Update the C1 commit
instructions to use git add -A instead of interactive git add -p, ensuring new
implementation and test files are staged non-interactively. Retain the
requirement to inspect the staged diff before running the existing commit
command.
Comment on lines
+367
to
+375
| ### C7: Exit the Worktree | ||
|
|
||
| Call `ExitWorktree` with `action: "keep"` to return the session to the original directory while | ||
| leaving the worktree and branch on disk — the branch is already pushed, but keeping the worktree | ||
| lets follow-up commits (e.g. review feedback) land without re-cloning or re-checking-out. | ||
|
|
||
| Only call `ExitWorktree` if this session entered the worktree via `EnterWorktree` in B2 (it is a | ||
| no-op otherwise). Mention to the user that the worktree can be removed later with `ExitWorktree | ||
| (action: "remove")` or `git worktree remove` once the PR merges. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Exit worktrees entered through Path C as well.
A direct review flow can enter the feature worktree in C0, but C7 only exits when B2 entered it. Track whether C0 or B2 attached the session and call ExitWorktree(action: "keep") for either path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/pick-story/SKILL.md` around lines 367 - 375, Update the worktree
tracking in the Path C review flow so C7 recognizes sessions entered through
either C0 or B2. Call ExitWorktree with action "keep" whenever either path
attached the session, while preserving the no-op behavior for sessions that
entered neither worktree path.
Comment on lines
+116
to
+129
| Run `git worktree list` and look for a path matching `plan/<story_key>`. | ||
|
|
||
| - **Found in the list (resume):** Call `EnterWorktree(path: <that path>)` to reattach. Then check | ||
| whether `_bmad-output/implementation-artifacts/<story_key>.md` already exists in the worktree: | ||
| - **It exists** — HALT and ask the user how to proceed: (a) reuse the existing story file and | ||
| skip directly to Step 4 (review), (b) delete it and regenerate via Step 3, or (c) abort. Never | ||
| silently overwrite or silently reuse — a resumed worktree may hold work the user still wants. | ||
| - **It doesn't exist** — continue normally to Step 3. | ||
| - **Not in the list, but the branch exists (orphaned worktree):** Check | ||
| `git branch --list plan/<story_key>` and `git branch -r --list origin/plan/<story_key>`. If | ||
| either finds the branch, the worktree was likely removed manually while the branch survived. | ||
| Run `git worktree add .claude/worktrees/plan/<story_key> plan/<story_key>` to recreate the | ||
| worktree from the existing branch, then `EnterWorktree(path: .claude/worktrees/plan/<story_key>)` | ||
| to attach. Then apply the same existing-file check as the resume case above. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Check resumed worktrees for dirty state before attaching.
Both workflows can reattach to an existing worktree before checking its working-tree state, risking modifications to another session’s uncommitted work.
skills/plan-story/SKILL.md#L116-L129: rungit status --porcelainbeforeEnterWorktreefor resume and orphaned planning worktrees; halt on conflicts.skills/pick-story/SKILL.md#L203-L210: apply the same cleanliness check before reattaching feature worktrees.
📍 Affects 2 files
skills/plan-story/SKILL.md#L116-L129(this comment)skills/pick-story/SKILL.md#L203-L210
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/plan-story/SKILL.md` around lines 116 - 129, Before each EnterWorktree
call in the resume and orphaned-worktree flows, run git status --porcelain for
the target worktree and halt if it reports uncommitted changes or other
conflicts; only attach when clean. Apply this to skills/plan-story/SKILL.md
lines 116-129 and skills/pick-story/SKILL.md lines 203-210, preserving the
existing reattachment and existing-file checks.
Comment on lines
+140
to
+142
| If the resulting worktree name would exceed the tool's 64-character limit, truncate the slug | ||
| portion (keep `plan/<epic>-<num>-` intact, cut from the end of the descriptive slug) and note the | ||
| shortened name when reporting to the user in Step 7. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply name truncation consistently.
If the worktree name exceeds 64 characters, this only says to truncate the slug, while later commands still use the full plan/<story_key> branch/path and reports use the unshortened key. Define one shortened identifier and use it consistently for the branch, worktree path, resume lookup, commit handoff, and cleanup.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/plan-story/SKILL.md` around lines 140 - 142, Update the worktree
naming flow in the plan story instructions to compute one shortened identifier
when the 64-character limit is exceeded, preserving the required prefix and
truncating only the descriptive slug. Use that identifier consistently for
branch creation, worktree paths, resume lookup, commit handoff, cleanup, and
Step 7 reporting instead of mixing shortened and full names.
This was referenced Jul 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation