Skip to content

feat(cli): polish self-update progress - #947

Open
benvinegar wants to merge 2 commits into
mainfrom
feat/clack-update-progress
Open

feat(cli): polish self-update progress#947
benvinegar wants to merge 2 commits into
mainfrom
feat/clack-update-progress

Conversation

@benvinegar

Copy link
Copy Markdown
Member

Summary

  • render hunk update as a compact Clack-guided flow in interactive terminals
  • animate the package-manager step while keeping its noisy output out of the transient display
  • preserve the existing stable plain-text contract for pipes, CI, NO_COLOR, and TERM=dumb
  • show captured installer output when an update fails

Preview

┌  Hunk update
│
●  Current  0.20.1
│
●  Target   0.21.0
│
◇  Updated to 0.21.0
│
└  Done

During installation, the status line animates in place as ◒ ◐ ◓ ◑ Updating with npm.

Validation

  • bun run typecheck
  • bun run lint
  • bun run deps:check
  • bun test src/core/install/selfUpdate.test.ts
  • bun run test — 2,136 passed, 3 skipped
  • bunx oxfmt --check src/core/install/selfUpdate.ts src/core/install/selfUpdate.test.ts src/main.tsx package.json .changeset/cool-hunk-update.md
  • git diff --check

Tested on Linux. Windows and macOS were not manually tested; the existing cross-platform command-selection coverage remains green.

This PR description was generated by Pi using gpt-5.6-sol

@vercel

vercel Bot commented Aug 31, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hunk-web Ignored Ignored Preview Aug 31, 2026 12:54pm

Request Review

@greptile-apps

greptile-apps Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a compact Clack-based presentation for interactive hunk update runs, captures installer output while the spinner is active, and preserves the existing plain path for most non-interactive environments.

  • Adds @clack/prompts and wires stdout terminal information through the CLI entry point.
  • Captures and replays package-manager output on interactive failures.
  • Adds tests for interactive success and truthy NO_COLOR fallback.

Confidence Score: 4/5

The plain-output predicate should be fixed before merging because documented color-disabled and CI invocations can receive animated terminal output.

The new renderer is selected for an empty NO_COLOR value and for CI jobs using a pseudo-terminal, despite the release contract requiring stable plain text in both cases.

Files Needing Attention: src/core/install/selfUpdate.ts, src/core/install/selfUpdate.test.ts

Important Files Changed

Filename Overview
src/core/install/selfUpdate.ts Adds interactive progress and captured child output, but its mode predicate fails to preserve plain output for empty NO_COLOR and pseudo-TTY CI environments.
src/core/install/selfUpdate.test.ts Covers interactive success and NO_COLOR="1", but does not exercise the two plain-output conditions missed by the predicate.
src/main.tsx Correctly supplies stdout and its TTY state to the self-update command, making the new presentation reachable from production.
package.json Adds the exact-pinned Clack dependency used by the new update presentation.
Prompt To Fix All With AI
### Issue 1
src/core/install/selfUpdate.ts:358
**Plain-output gating ignores environment**

When `NO_COLOR` is present with an empty value, this truthiness check still enables the Clack renderer. If a CI job allocates a pseudo-terminal, the missing CI guard does the same, causing animated terminal control output to replace the documented stable plain-text output in captured logs.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "feat(cli): polish self-update progress" | Re-trigger Greptile

Comment thread src/core/install/selfUpdate.ts Outdated
const alreadyCurrent = input.version
? installedVersion === targetVersion
: !isComparableVersion(installedVersion) || !isNewerVersion(installedVersion, targetVersion);
const interactive = Boolean(io.stdoutIsTTY && io.output && !env.NO_COLOR && env.TERM !== "dumb");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Plain-output gating ignores environment

When NO_COLOR is present with an empty value, this truthiness check still enables the Clack renderer. If a CI job allocates a pseudo-terminal, the missing CI guard does the same, causing animated terminal control output to replace the documented stable plain-text output in captured logs.

Knowledge Base Used: Application runtime and startup

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/core/install/selfUpdate.ts
Line: 358

Comment:
**Plain-output gating ignores environment**

When `NO_COLOR` is present with an empty value, this truthiness check still enables the Clack renderer. If a CI job allocates a pseudo-terminal, the missing CI guard does the same, causing animated terminal control output to replace the documented stable plain-text output in captured logs.

**Knowledge Base Used:** [Application runtime and startup](https://app.greptile.com/modem/-/custom-context/knowledge-base/modem-dev/hunk/-/docs/application-runtime.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

@benvinegar benvinegar Aug 31, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 87799705. Interactive rendering now requires that neither CI nor NO_COLOR is present, including empty values, and regression coverage exercises both cases. I also hardened Clack cancellation so SIGINT/SIGTERM terminate the full installer process tree and return the corresponding shell status instead of allowing an update to continue.

Responded by Pi using gpt-5.6-sol.

This comment was generated by Pi using gpt-5.6-sol

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