Skip to content

fix(cli): resolve skill loading indicator line duplication#71

Merged
dcramer merged 1 commit intomainfrom
fix/cli-skill-loading-indicator-duplication
Feb 1, 2026
Merged

fix(cli): resolve skill loading indicator line duplication#71
dcramer merged 1 commit intomainfrom
fix/cli-skill-loading-indicator-duplication

Conversation

@dcramer
Copy link
Member

@dcramer dcramer commented Feb 1, 2026

  • Fixed CLI duplicating skill loading indicator lines for each file being processed
  • Root causes: multiple Static components with absolute positioning, unstable item references, unbatched rapid rerenders
  • Added comprehensive documentation explaining Ink rendering constraints for future maintainers

The CLI was duplicating skill loading indicator lines for each file
being processed due to three issues with Ink rendering:

1. Multiple Static components with absolute positioning caused layout
   conflicts. Fixed by printing the header before Ink starts and using
   a single Static component for completed items.

2. Wrapping items in new objects on each render broke Static's
   reference equality tracking. Fixed by passing items directly
   without wrapper objects.

3. Rapid consecutive rerender() calls from file update callbacks
   weren't being batched. Fixed by coalescing updates using
   setImmediate().

Also added comprehensive documentation explaining these Ink rendering
constraints for future maintainers.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Feb 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
warden Ready Ready Preview, Comment Feb 1, 2026 6:50pm

Request Review

Comment on lines +237 to 245
let updatePending = false;
const updateUI = () => {
rerender(<SkillRunner skills={[...skillStates]} completedItems={[...completedItems]} />);
if (updatePending) return;
updatePending = true;
setImmediate(() => {
updatePending = false;
rerender(<SkillRunner skills={[...skillStates]} completedItems={[...completedItems]} />);
});
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug fix missing regression test (high confidence)

This PR fixes a bug (skill loading indicator line duplication) but adds no regression test. Per testing guidelines, bug fixes must include a test that would have caught the issue.


warden: testing-guidelines

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

setImmediate(() => {
updatePending = false;
rerender(<SkillRunner skills={[...skillStates]} completedItems={[...completedItems]} />);
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rerender may be called after Ink unmount

Low Severity

The batching mechanism using setImmediate() creates a race condition where rerender() may be called after unmount(). When a callback fires just before task completion, it schedules a deferred rerender. The await then resolves and unmount() is called synchronously, but the setImmediate callback runs afterward and attempts to call rerender() on the unmounted Ink instance. The updatePending flag does not account for the unmounted state.

Additional Locations (1)

Fix in Cursor Fix in Web

@dcramer dcramer merged commit c7862d8 into main Feb 1, 2026
11 of 14 checks passed
@dcramer dcramer deleted the fix/cli-skill-loading-indicator-duplication branch February 1, 2026 18:58
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