Skip to content

fix(pipeline): register pipeline-volume-cover-concepts stage#262

Merged
atomantic merged 2 commits into
mainfrom
worktree-fix-volume-cover-concepts-stage
May 17, 2026
Merged

fix(pipeline): register pipeline-volume-cover-concepts stage#262
atomantic merged 2 commits into
mainfrom
worktree-fix-volume-cover-concepts-stage

Conversation

@atomantic
Copy link
Copy Markdown
Owner

Summary

  • The per-season volume cover-concept LLM step (added in d802eb1) shipped its prompt template at data.sample/prompts/stages/pipeline-volume-cover-concepts.md but forgot to register the stage in stage-config.json and forgot to add an upgrade migration.
  • Upgraded installs hit ❌ Route error: Stage pipeline-volume-cover-concepts not found from arcPlanner.generateVolumeCoverConceptsrunStagedLLM('pipeline-volume-cover-concepts', …)prompts.buildPrompt().
  • Adds the missing config entry and a migration (017-volume-cover-concepts-stage.js) that seeds both the .md template and the stage-config entry on existing installs.

Why fresh installs weren't affected

  • setup-data.js's ensureSampleContent already copies missing prompt files from data.sample/ on fresh setup — so the .md template landed correctly.
  • The stage-config entry however is only merged via JSON_MERGE_TARGETS on first-time setup, never on upgrade. That's the hole this migration plugs.

Changes

  • data.sample/prompts/stage-config.json — add pipeline-volume-cover-concepts entry (model: "default", returnsJson: true, no shared variables — matches sibling per-season prompts like pipeline-season-episodes).
  • scripts/migrations/017-volume-cover-concepts-stage.js — new migration; idempotent; mirrors the structure of 015-importer-stage-prompts.js. Seeds the .md template if missing, then merges the stage-config entry if missing.
  • .changelog/NEXT.md — Fixed entry.

Test plan

  • Migration runs cleanly on a fresh empty data/ dir (seeds both .md and config entry)
  • Migration re-run is a no-op when both already present (prints "already present" for each)
  • data.sample/prompts/stage-config.json still parses as valid JSON
  • After merge: user restarts server with npm run install:all (which invokes scripts/run-migrations.js); migration 017 applies; generateVolumeCoverConcepts route no longer 500s

🤖 Generated with Claude Code

The per-season volume cover-concept LLM step shipped in d802eb1 added
the prompt template but missed the stage-config entry and a migration
for existing installs, so upgraded installs hit "Stage pipeline-volume-cover-concepts
not found" when arc planner tries to generate covers.

- Add the stage-config entry to data.sample/prompts/stage-config.json
- Add migration 017 that seeds the template + config entry on upgrade
Copilot AI review requested due to automatic review settings May 17, 2026 12:12
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

Registers the missing pipeline-volume-cover-concepts LLM stage for upgraded installs by adding the stage entry to the sample stage-config.json and introducing a migration that seeds both the prompt template and the config entry when absent.

Changes:

  • Add pipeline-volume-cover-concepts to data.sample/prompts/stage-config.json.
  • Add migration 017-volume-cover-concepts-stage.js to seed the stage prompt template and stage-config entry on existing installs.
  • Update .changelog/NEXT.md to document the fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
scripts/migrations/017-volume-cover-concepts-stage.js New migration to copy the prompt template (if missing) and merge the stage-config entry (if missing).
data.sample/prompts/stage-config.json Registers the pipeline-volume-cover-concepts stage definition for new installs and as a source for migrations.
.changelog/NEXT.md Adds a “Fixed” entry describing the upgraded-install 500 fix.

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

Comment thread scripts/migrations/017-volume-cover-concepts-stage.js
Comment thread scripts/migrations/017-volume-cover-concepts-stage.js Outdated
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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@atomantic atomantic merged commit 7018db0 into main May 17, 2026
6 checks passed
@atomantic atomantic deleted the worktree-fix-volume-cover-concepts-stage branch May 17, 2026 12:20
atomantic added a commit that referenced this pull request May 17, 2026
* fix(boot): run pending data migrations on every server start

The migration runner was only invoked from update.sh — plain pull-and-restart
(or pm2 restart after a manual git pull) left shipped migrations unapplied,
so new prompt stages, settings renames, and seeded data sat in the queue
until the user ran 'npm run update' or 'npm run migrations'. Existing installs
hit cryptic 'Stage X not found' errors despite the fix shipping in main
(see PR #262 / pipeline-volume-cover-concepts).

Refactored scripts/run-migrations.js to export a runMigrations() function
and kept the CLI shim. server/index.js now awaits the runner before the AI
toolkit reads stage-config.json. Idempotent and cheap when the applied list
is current; logs and continues on failure rather than refusing to boot.

* address review: resolve CLI argv path before pathToFileURL

`pathToFileURL()` throws on relative paths, which can happen when the
script is launched as `node scripts/run-migrations.js`. Resolve argv[1]
to an absolute path first, then realpath both sides to follow npm bin
shims / symlinks reliably.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* address review: synchronous CLI guard + full-stack error logs

- scripts/run-migrations.js: drop the async realpath-based isInvokedAsScript
  helper that turned the module into an async module and forced
  filesystem I/O at evaluation time. Switch to a synchronous
  pathToFileURL(resolve(argv[1])) compare against import.meta.url, which
  still handles the relative-argv case the original review flagged.
- scripts/run-migrations.js + server/index.js: log err?.stack ?? err on
  migration failures so boot diagnostics include the full trace and
  tolerate non-Error throws.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* address review: recover from corrupt migrations.applied.json

A crash mid-write to data/migrations.applied.json can leave the file
truncated or malformed, which previously caused every subsequent boot
to hard-fail. Detect bad JSON or non-array shapes, rename the corrupt
file aside (.corrupt-<ISO timestamp>) for forensics, and rebuild from
scratch — migrations are idempotent so re-running them is safe.

Also parameterize migrationsDir so runMigrations is testable against
a fixture migrations directory, and broaden the vitest include glob
to pick up scripts/*.test.js alongside scripts/migrations/*.test.js.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
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.

2 participants