Skip to content

fix(tracker): drain item description into worktree on first attach - #227

Merged
agent-era-ai merged 4 commits into
mainfrom
requirements-in-worktree
Apr 26, 2026
Merged

fix(tracker): drain item description into worktree on first attach#227
agent-era-ai merged 4 commits into
mainfrom
requirements-in-worktree

Conversation

@agent-era-ai

Copy link
Copy Markdown
Collaborator

Summary

  • createItem no longer writes any files to <projectPath>/tracker/items/<slug>/. The user-typed description is stashed on index.sessions[slug].description and drained into <worktree>/tracker/items/<slug>/notes.md on the first ensureItemFiles call.
  • No more requirements.md stub anywhere — that file only appears when the requirements stage produces real content. Existing orphan probes (!currentItem.requirementsPath) keep working because they check the path string, not file existence.
  • ensureItemFiles migrates files from any legacy main-project location (the old tracker/items/<slug>/ staging dir, plus pre-refactor tracker/{backlog,implementation}/<slug>/ layouts) and deletes the source dir so the project root gets cleaned up.
  • readItem title fallback now reads index.sessions[slug].title so the kanban renders the correct title even without the stub.

Test plan

  • npm run typecheck clean
  • npm test — 724/724 pass
  • Updated tests in tests/unit/tracker.test.ts, tests/unit/tracker-board-create-flow.test.ts, tests/unit/tracker-proposal-create.test.ts to reflect index-only createItem and ensureItemFiles doing the file writes
  • Smoke check: create an item via the kanban with a description, attach to its worktree, confirm notes.md lands in the worktree (not the project root) and index.sessions[slug].description is cleared
  • Smoke check: open an existing pre-fix item, confirm legacy <projectPath>/tracker/items/<slug>/ is migrated into the worktree and the source dir is removed

🤖 Generated with Claude Code

agent-era-ai and others added 4 commits April 26, 2026 03:33
createItem no longer writes any files to <projectPath>/tracker/items/<slug>/.
The user-typed body is stashed on index.sessions[slug].description, and
ensureItemFiles drains it into notes.md inside the worktree on first run.
No requirements.md stub is created — that file only appears when the
requirements stage produces real content. Legacy main-project source dirs
are still migrated, then deleted so the project root gets cleaned up.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Use leading-underscore parameter and destructure variable names so TS
sees them as intentionally unused, instead of `void item;` /
`void _drop;` noise lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fuse readSessionDescription + clearSessionDescription into one read-modify-
write inside ensureItemFiles. Each cold-path call now reads the index at
most once and only writes when a description was actually stashed; old
shape did two reads (one always wasted) and one wasted write per attach.

Drop the now-unused TrackerItem parameter from ensureItemFiles; readItem
already reads the title from index.sessions[slug].title.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Apr 26, 2026

Copy link
Copy Markdown

Code Review: fix(tracker): drain item description into worktree on first attach

Overview

This PR fixes a design flaw where createItem was writing files (requirements.md, notes.md) directly into the project root's tracker/items/<slug>/ directory, rather than waiting until a worktree existed. The new approach defers all file I/O to ensureItemFiles, stashing the description on index.sessions[slug].description in the meantime. Legacy main-project staging dirs are migrated and deleted on first attach.

This is a clean, well-reasoned architectural fix. The logic is sound and the tests are thorough. A few issues worth flagging:


Potential Bug: delete-after-copy on same-path edge case

In ensureItemFiles (TrackerService.ts ~line 674–681):

for (const src of legacySources) {
  for (const file of fs.readdirSync(src)) {
    const destFile = path.join(destDir, file);
    if (fs.existsSync(destFile)) continue;
    fs.copyFileSync(path.join(src, file), destFile);
    wroteAnything = true;
  }
  fs.rmSync(src, {recursive: true, force: true});  // ← deletes immediately after copy
}

legacySources includes path.join(mainProjectPath, 'tracker', 'items', slug) and destDir is path.join(worktreePath, 'tracker', 'items', slug). If worktreePath === mainProjectPath (unusual but possible in a testing or single-checkout scenario), src === destDir and the fs.rmSync would delete the files that were just "copied" to themselves. A guard like if (src === destDir) continue; before fs.rmSync would eliminate this risk.


Potential Data Loss: description silently discarded when notes.md already exists from legacy migration

The comment acknowledges this is intentional, but it creates a silent drop scenario:

if (!fs.existsSync(notesPath)) {
  fs.writeFileSync(notesPath, `${entry.description}\n`);
  wroteAnything = true;
}
// description cleared regardless
const {description: _, ...rest} = entry;

If a legacy notes.md was migrated and the user had typed a new description (e.g., they re-created an item over an old one), the new description is silently discarded. The index is also cleared, so there's no recovery path. Consider logging a warning or appending the new description to the existing file, or at minimum noting this in the comment.


Tracker item file included in PR diff

tracker/items/requirements-in-worktree/requirements.md is included in the diff — this is the tracker item for the bug being fixed, written using the old behavior (to the project root). It's a small irony: the PR that fixes files landing in the project root includes such a file. Not a blocker, just worth noting for future PRs using the new flow.


Style / Minor

  • The new comment block in createItem explaining the stash-and-drain contract is detailed and useful. Good documentation of non-obvious behavior.
  • readItem title fallback (index.sessions?.[slug]?.title) is a clean fix and keeps the kanban correct before the worktree is attached.
  • Removing writeRequirementsStub and the stub concept entirely is a good simplification; no dead code left behind.
  • Test renames accurately reflect the new semantics (e.g., "stashes on the index for the worktree to drain" vs the old file-path assertions).

Test Coverage

The new tests are solid:

  • writes notes.md (no requirements.md stub) when sessions[slug].description is set
  • clears sessions[slug].description from the index after draining
  • writes nothing when there is no description and no legacy source
  • does not overwrite existing notes.md in worktree
  • migrates legacy main-project bucket files into the worktree and deletes the source dir

Missing test: the edge case where legacy migration populates notes.md and a description is present — verifying the description is cleared without being written (since the current behavior silently drops it). This would make the intentional data-drop behavior explicit and visible in CI.


Summary: The core logic and architecture are correct. The delete-after-copy on same-path and the silent description-discard are the two items worth addressing before merging.

@agent-era-ai

Copy link
Copy Markdown
Collaborator Author

Thanks for the review. Triaged:

  1. Same-path deleteworktreePath === mainProjectPath isn't reachable via either caller (App.tsx passes a real worktree path; tests use mkdtempSync). Adding a guard for an unreachable input would be defensive code for a scenario that can't happen, so leaving it.
  2. Silent description-discard with legacy notes.mdcreateItem early-returns for same-slug-same-stage, so the only way to reach the silent-drop path is recreating an item with the same slug at a different stage while a pre-fix legacy <projectPath>/tracker/items/<slug>/ still exists. That's a corner that effectively can't be hit through the kanban; the existing inline comment documents the deliberate clear-on-visit. Leaving it.
  3. Tracker item file in diff — agreed, fair to flag. The file was already in the tree from the prior seed commit; the modification is the actual stage output for this item, which is fine to keep.

@agent-era-ai
agent-era-ai merged commit d5e947b into main Apr 26, 2026
2 checks passed
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.

1 participant