Skip to content

fix: surface vp install errors outside collapsed log group#52

Merged
fengmk2 merged 2 commits intomainfrom
fix/surface-vp-install-errors
Apr 22, 2026
Merged

fix: surface vp install errors outside collapsed log group#52
fengmk2 merged 2 commits intomainfrom
fix/surface-vp-install-errors

Conversation

@fengmk2
Copy link
Copy Markdown
Member

@fengmk2 fengmk2 commented Apr 22, 2026

Summary

When vp install fails inside the action (e.g. ERR_PNPM_OUTDATED_LOCKFILE, yarn integrity mismatch), the actual error detail is hidden inside a collapsed GitHub Actions log group. GitHub never auto-expands a group on failure, so users see only ▶ Running vp install in ... and have to click to find out why their workflow broke. Reported from two real CI runs — see nkzw-tech/web-app-template run and fengmk2/fengmk2.github.com run.

  • Capture stdout/stderr via exec listeners while the command runs (live output still streams into the group).
  • On non-zero exit: close the group before logging so the failure renders at top level, and emit the tail of the captured output via core.error(...) so it also appears in the run's Annotations panel. Tail-capped at 4000 chars.
  • Success path is unchanged — still tidy inside the group.

Only runViteInstall used a log group; every other step in this action (installVitePlus, vp env use, restoreCache, saveCache) already logs at top level, so this brings vp install in line with the rest.

Test plan

  • vp run typecheck
  • vp run check:fix
  • vp run test (97 passing)
  • vp run build (dist regenerated)
  • E2E smoke test: run a workflow pinned to this branch against a repo with an intentionally out-of-sync lockfile; confirm the installer error is visible at top level and appears in the Annotations panel without expanding the group.
  • E2E smoke test: run against a repo with a clean install; confirm the success path still groups cleanly and logs "Successfully ran vp install".

Note

Low Risk
Low risk: changes are limited to logging/error-reporting around vp install, but could slightly alter how failures are surfaced and grouped in GitHub Actions logs.

Overview
Ensures vp install failures are visible at the top level of GitHub Actions logs (and in the Annotations panel) instead of being hidden inside a collapsed log group.

runViteInstall now runs vp install via getExecOutput(..., ignoreReturnCode: true), closes the log group before emitting errors, and logs a truncated tail (up to 4000 chars) of stdout/stderr via core.error before calling setFailed with the exit code.

Reviewed by Cursor Bugbot for commit 697e70a. Configure here.

When `vp install` failed inside the action, the error detail (e.g.
`ERR_PNPM_OUTDATED_LOCKFILE`) was hidden inside a collapsed GitHub
Actions log group because GitHub never auto-expands groups on failure.
Users had to click the `▶ Running vp install in ...` line to find out
why their workflow broke.

Capture stderr (falling back to stdout) while the command runs, then on
non-zero exit code close the group before logging so the failure is
rendered at top level and also emit a `core.error` annotation with the
tail of the output so it appears in the run's Annotations panel. Live
output still streams into the group during the run, and the success
path is unchanged.
Copilot AI review requested due to automatic review settings April 22, 2026 11:14
@fengmk2 fengmk2 self-assigned this Apr 22, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Surfaces vp install failure details outside the collapsed GitHub Actions log group so users can immediately see installer errors (and get an annotation) without expanding grouped logs.

Changes:

  • Capture vp install stdout/stderr while running.
  • On non-zero exit, close the log group before emitting error details and add a capped tail of output via core.error(...) annotations.
  • Keep success-path logging grouped and unchanged.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/run-install.ts Captures install output and emits a top-level error/annotation with truncated details on failure.
dist/index.mjs Regenerated build output to include the updated install logging behavior used by the action runtime.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/run-install.ts Outdated
Comment on lines +26 to +42
let stderrBuffer = "";
let stdoutBuffer = "";
let groupOpen = true;

startGroup(`Running ${cmdStr} in ${cwd}...`);

try {
const exitCode = await exec("vp", args, {
cwd,
ignoreReturnCode: true,
listeners: {
stdout: (data: Buffer) => {
stdoutBuffer += data.toString();
},
stderr: (data: Buffer) => {
stderrBuffer += data.toString();
},
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

stdoutBuffer/stderrBuffer accumulate the full vp install output with no upper bound. Install logs can be very large, so this can cause unnecessary memory growth inside the action runner. Consider keeping only a rolling tail (e.g., cap each buffer to MAX_ERROR_TAIL (or a small multiple) inside the listeners) since only the tail is used on failure anyway.

Copilot uses AI. Check for mistakes.
Replace the hand-rolled `exec` + `listeners.stdout/stderr` buffer
capture with `getExecOutput`, which already returns
`{ exitCode, stdout, stderr }` and still streams live output into the
group. Drop the `groupOpen` boolean (`endGroup()` is idempotent — just
writes `::endgroup::`, repeated calls are harmless), remove the
`finally` block, and flatten the success path with an early `continue`.
fengmk2 added a commit to fengmk2/fengmk2.github.com that referenced this pull request Apr 22, 2026
Use the commit SHA at the head of PR #52 ("fix: surface vp install
errors outside collapsed log group") so install failures on CI/deploy
show up outside the collapsed setup-vp log group. Revert to @v1 once
the PR merges.
@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 22, 2026

@fengmk2
Copy link
Copy Markdown
Member Author

fengmk2 commented Apr 22, 2026

@cursor review

Copy link
Copy Markdown

@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.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 697e70a. Configure here.

@fengmk2 fengmk2 merged commit e1660b7 into main Apr 22, 2026
42 checks passed
@fengmk2 fengmk2 deleted the fix/surface-vp-install-errors branch April 22, 2026 11:43
fengmk2 added a commit to fengmk2/fengmk2.github.com that referenced this pull request Apr 22, 2026
* Migrate blog to Void (Vite + Cloudflare Workers + Vue 3)

Replace legacy Node.js build (build.js + mdit) with Void framework.
Blog markdown files become first-class page routes via @void-x/md.
Static assets moved to public/ for Void's static file serving.

- Add vite.config.ts with voidPlugin, voidVue, voidMarkdown
- Create pages/layout.vue (shared) and pages/blog/layout.vue (blog chrome + Disqus)
- Create pages/index.vue homepage with all curated content
- Move 111 blog .md files to pages/blog/
- Add middleware/html-compat.ts for .html → clean URL redirects
- Copy static assets (css, js, images, ppt, collections, etc.) to public/
- Remove scaffolded artifacts (migrations, routes/api, src/demo.ts)
- Update package.json deps, tsconfig.json, deploy.yml branch

* Replace GitHub ribbon with Deploy on V___ ribbon

- Update layout.vue: diagonal ribbon style replacing fork-me image
- Remove original blog/ directory (content moved to pages/blog/ and public/blog/)
- Remove root index.html (replaced by pages/index.vue)
- Remove shadow HTML files from public/blog/ that have .md page equivalents

* Fix deploy: move legacy files, fix SSR CSS and Disqus scripts

- Move legacy root-level directories to legacy.bak/ to reduce deploy
  size and prevent Vite from bundling them into SSR
- Fix CSS causing worker deploy failure by moving @import to <link>
  tags and using <component is="style"> for custom styles
- Fix Disqus comments not loading by using <component is="script">
  instead of onMounted (no client hydration in SSR pages)
- Change image src to dynamic :src bindings to avoid Vite asset
  processing
- Restore hangjs-family-photo2.jpg resized to 2000px (179KB vs 5.6MB)

* Fix ppt deck.js paths to use relative URLs

Replace absolute https://fengmk2.com:9443/ references with relative
paths so slides work on any domain.

* Return 404 for unknown paths and add sitemap generator

Remove middleware/html-compat.ts which unconditionally stripped .html
from any request path and issued a 301 without checking that the
resource existed. Crawlers hitting garbage concatenated paths like
/ppt/.../README.html were getting 301s that search engines then
cached. Without the middleware, real .html files in public/ still
serve 200 via env.ASSETS, and non-existent paths fall through to 404.

Add public/sitemap.xml + public/robots.txt so crawlers can discover
the canonical URL set instead of guessing. scripts/generate-sitemap.mjs
walks pages/blog/**/*.md and regenerates the sitemap; wired into
package.json as prebuild so every vite build refreshes it.

* Switch void packages to @void-sdk GitHub Packages aliases

Replace local file: paths with the official npm aliases documented in
the void-sdk/void README:

- void@npm:@void-sdk/void@^0.5.0
- @void/vue@npm:@void-sdk/vue@^0.4.0
- @void/md@npm:@void-sdk/md@^0.5.0

Rename imports in vite.config.ts from @void-x/* to @void/* and drop
the resolve.alias shim that mapped between the two scopes. Switch
packageManager to pnpm@10.33.0 per the README's recommended tooling
and add pnpm-lock.yaml.

* update deps

* Add PR staging deploy workflow, switch deploy to pnpm

Add .github/workflows/ci.yml adapted from voidzero-dev/setup.viteplus.dev:
- test job on every push to master and every PR (pnpm install + build)
- staging-deploy job on PRs only, deploys to VOID_PROJECT=fengmk2-staging
  and posts/updates a sticky comment with the preview URL

Update .github/workflows/deploy.yml to use pnpm instead of npm ci,
matching the packageManager switch to pnpm@10.33.0. Production deploy
continues to read the projectId from .void/project.json.

Both workflows use the latest action versions: actions/checkout@v6,
actions/setup-node@v6, actions/github-script@v9, pnpm/action-setup@v5.
Installation auths against npm.pkg.github.com via NODE_AUTH_TOKEN so
@void-sdk/* packages resolve.

* ignore .void

* Switch CI/deploy workflows to voidzero-dev/setup-vp

Replace the pnpm/action-setup + actions/setup-node pair with the
official setup-vp action, matching the voidzero-dev/setup.viteplus.dev
reference. setup-vp handles Node install via 'vp env use',
auto-detects pnpm from the lockfile, and caches the pnpm store.

Replace 'pnpm build' with 'vp run build' and 'pnpm exec void deploy'
with 'vpx void deploy' so the workflow is package-manager-agnostic
and relies on Vite+'s own script runner.

Install auth against npm.pkg.github.com still flows through
NODE_AUTH_TOKEN; setup-vp's default run-install uses that env to pull
@void-sdk/* packages. Production and staging projects resolved
exactly as before.

* Migrate project to Vite+ (vp)

Run 'vp migrate --no-interactive' to adopt Vite+ as the unified
toolchain (runtime, package manager, dev/build, lint, format).

- vite.config.ts now imports from 'vite-plus'; drops the Vite-only
  shape for Vite+'s defineConfig with 'staged', 'fmt', 'lint' blocks
- package.json scripts use 'vp dev / vp build / vp preview'; add
  'prepare: vp config' for the pre-commit hook setup
- pnpm-workspace.yaml added with catalog routing 'vite' to
  '@voidzero-dev/vite-plus-core' and 'vitest' to 'vite-plus-test';
  peerDependencyRules relax matching so void plugins keep resolving
- env.d.ts adds the '*.vue' TypeScript shim
- AGENTS.md captures the Vite+ workflow for coding agents
- .vite-hooks/pre-commit wires staged-file checks via 'vp staged'

Follow-up tweaks on top of the auto-migration:

- Legacy content at the repo root (blog.bak, legacy.bak, MOVE,
  movement.js, max_new_space_size, libuv, scattered *.md/*.js/*.html)
  is added to 'fmt.ignorePatterns' and 'lint.ignorePatterns' in
  vite.config.ts so 'vp check' only looks at the active source tree.
- Format-only touch-ups on CLAUDE.md, env.d.ts, tsconfig.json,
  package.json, scripts/generate-sitemap.mjs, pages/*.vue,
  pnpm-workspace.yaml, vite.config.ts, and .void/entry.ts produced
  by 'vp fmt --write'.
- CI: add 'vp check' as a pipeline gate before 'vp run build'.
  'vp build' skips npm lifecycle hooks, so both CI and deploy
  workflows now run 'vp run sitemap' before 'vpx void deploy' to
  keep public/sitemap.xml fresh on every deploy.

Verification:
- vp install: ok
- vp check:   ok (18 files formatted, 5 files lint/typecheck clean)
- vp build:   ok (162 modules, ~750ms)
- vp test:    fails at config resolution because
  '@cloudflare/vite-plugin' rejects the SSR environment's
  'resolve.external' set by voidPlugin() when vitest loads the
  config. No tests exist, so this is non-blocking; revisit when
  tests are added (likely needs a vitest-scoped config variant).

* fmt

* Pin setup-vp to voidzero-dev/setup-vp#52

Use the commit SHA at the head of PR #52 ("fix: surface vp install
errors outside collapsed log group") so install failures on CI/deploy
show up outside the collapsed setup-vp log group. Revert to @v1 once
the PR merges.

* Bump setup-vp PR #52 pin to latest head

21f6c6e -> 697e70a (new commit pushed to fix/surface-vp-install-errors).

* Refactor setup-vp usage to PR #54 latest behavior

Bump setup-vp pin from e79fc32 to af4ffd9 (new head of PR #54) and
drop the repo .npmrc auth-line dance. The updated PR now auto-
generates _authToken entries at \$RUNNER_TEMP/.npmrc for every
registry declared in the repo .npmrc when NODE_AUTH_TOKEN is set,
so the committed .npmrc can stay as just the registry mapping.

Also update CLAUDE.md guidance to match the new pattern.

* Bump setup-vp PR #54 pin to latest head (af4ffd9 -> 42d342a)

* Pin setup-vp to the merged commit of PR #54

42d342a (PR branch head) -> 4f5aa3e (squash-merge on main). PR #54
is now merged into voidzero-dev/setup-vp main with title
"feat: respect project .npmrc and auto-generate _authToken".

* Use voidzero-dev/setup-vp@v1 floating tag

* Fixup

* Serve a real 404 page for unknown URLs

Void's default notFound handler renders the index component with status
404, so unknown URLs looked like the homepage. Add a catch-all
pages/[...slug] that renders a dedicated Not Found page, and a global
middleware that rewrites the response status to 404 because Void's
renderedResponse builds a fresh Response without honoring c.status() set
inside a loader.

* Give every page a <title>

Void only auto-derives a head title from YAML frontmatter (not from the
first H1), and Vue pages need an explicit head() export. Add titles for
the home page and the 404 page, and inject title: frontmatter into all
markdown blog posts that lacked it (derived from each post's first H1).
The new scripts/inject-md-titles.mjs is the one-off generator.

* Log user-agent and client IP on 404 hits

Helps spot scanners and broken inbound links from runtime logs. IP
prefers cf-connecting-ip on Cloudflare and falls back to forwarded
headers in other environments.
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.

Improve vp install GitHub Actions error output so failures are obvious immediately

2 participants