fix: generated projects are broken — repair the generator, harden secrets, prove it in CI - #1
Open
tashikomaaa wants to merge 7 commits into
Open
tashikomaaa wants to merge 7 commits into
tashikomaaa wants to merge 7 commits into
Conversation
Relation fragments (Mongoose field definitions, GraphQL fields, the
populate chain) were interpolated with EJS `<%= %>`, which HTML-escapes
its output. Every quote in the generated code became `'`:
author: { type: mongoose.Schema.Types.ObjectId, ref: 'User' },
so `aapi generate model <Name> --relations ...` produced a model and a
resolver that fail to parse, and the scaffolded project could not start.
These fragments are code, not user-facing text, so they are now injected
with `<%- %>`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW
Several `aapi generate model` flags produced code that could not run: - `--with-subscriptions` / `--with-cache` generated a resolver importing `../../utils/pubsub.js` and `../../utils/cache.js`, but no command ever wrote those files. They are now rendered next to the resolver that needs them, and never overwrite an existing file. - `--no-tests` had no effect: commander maps it to `options.tests`, while the code read `options.withTests`, which is always undefined from the CLI. - Template paths were resolved with `new URL(import.meta.url).pathname`, which yields a broken path on Windows. Now uses `fileURLToPath`, as `create` already did. The two runtime modules were themselves unrunnable, which is why nobody noticed they were missing: - both called `require()` from an ES module (ReferenceError as soon as REDIS_URL was set); they now build one via `createRequire`. - `pubsub.js` imported `PubSub` from `graphql-yoga`, which v5 does not export — the API is `createPubSub`. - `cacheResolver` read `context.skipCache` unconditionally and crashed whenever a resolver was called without a context, as tests do. - the memory-cache cleanup interval was never `unref()`ed, so it kept the process (and Jest workers) alive forever. Subscriptions depend on the Yoga PubSub and Apollo Server needs transport wiring AAPI does not scaffold, so `--with-subscriptions` now fails with an actionable message on a non-Yoga project instead of emitting dead imports. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW
Tests could never run. The generated test file lands in `src/__tests__/` but imported `../../models/<Name>.js`, one level too high; `jest.config.js` and `jest.setup.js` existed as templates but were never copied into the project; the default package template declared no `test` script and no Jest dependency, while the secure one used plain `jest`, which cannot load ES modules without `--experimental-vm-modules`. Generated projects now ship a working Jest setup backed by an in-memory MongoDB, so `npm test` passes on a freshly scaffolded project. The setup falls back to MONGODB_URI_TEST when `mongodb-memory-server` is absent. When `--with-cache` is on, the generated tests clear the cache between cases: cached results otherwise outlive the documents they were built from and the suite fails on stale data. Two server-side defects: - `typeDefs/index.js` and `resolvers/index.js` globbed from `process.cwd()`, so the schema was empty unless the server was started from the project root — breaking Docker, PM2 and monorepo layouts. Both now resolve from their own module location. - `/health` was registered as a *second* `request` listener on a server already handled by Yoga. Both listeners ran for every request and the second one threw ERR_HTTP_HEADERS_SENT, killing the process on the first health check — exactly what a load balancer probe does. Health and Yoga now share one listener, and /health answers 503 when MongoDB is disconnected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW
…and version - `aapi create` never ran `npm install`. The `--skip-install` flag only changed which hint was printed, so it skipped nothing. Dependencies are now installed by default and the flag genuinely opts out; a failed install reports the tail of npm's output instead of failing silently. - `--secure --apollo` silently produced a non-secure Apollo project: security templates are Yoga-only. The combination now warns and says how to get the secure template, and the security files are no longer written next to a server that never loads them. - `.gitignore` and `.editorconfig` templates never reached scaffolded projects installed from npm: npm always strips files named `.gitignore`, and `.npmignore` excludes `.editorconfig`. Both templates are stored undotted and renamed on copy. A missing template file is now an error rather than a silent skip. - `aapi list` printed the *schema's* existence on the "Model" line, so a model file deleted by hand still showed as present. - `aapi --version` was hardcoded to 0.1.0 and would drift from package.json; it now reads the real version. `init` also learns the jest wiring (config files, `test` script and dev dependencies) so `aapi init` and `aapi create` produce the same setup, and its Apollo dependency list is updated alongside the Apollo Server 4 migration in the following commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW
`--secure` projects fell back to a hardcoded signing key when the
environment was not configured:
process.env.ACCESS_TOKEN_SECRET || 'your-access-secret-change-in-production'
That placeholder ships in this repository, so an API deployed without a
.env signed its tokens with a publicly known key: anyone could forge an
admin token. The failure was silent, which is the worst property a
secret-handling default can have.
The generated server now refuses to start in production when a secret is
missing, still the scaffolded placeholder, shorter than 32 characters, or
identical between access and refresh tokens (which would let a refresh
token be replayed as an access token). Outside production it warns on
every start instead of blocking local work.
Also migrates the `--apollo` path off `apollo-server-express` v3, which
reached end of life in October 2023 and receives no security patches, to
`@apollo/server` v4 with `@as-integrations/express4`. The Apollo package
template picks up the `test` script and Jest dependencies introduced
earlier in this branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW
AAPI is a code generator, but nothing checked its output. Unit tests over template inputs cannot see an EJS escaping bug, a missing import target or a server that dies on its own health check — all three shipped. Two layers: - `__tests__/commands/generate.integration.test.js` scaffolds projects in a temp directory and asserts every generated file parses under `node --check`, plus targeted guards for the regressions just fixed (escaped quotes, emitted runtime modules, `--no-tests`, test import depth). Verified by mutation: reverting the `<%-` fix makes it fail. - `scripts/verify-generated-project.mjs` goes further and is what CI now runs for both server flavours: scaffold, generate, type-check, install, run the project's own suite, boot the server **from `/`** against an in-memory MongoDB, hit /health twice and do a CRUD round-trip over GraphQL. Starting outside the project root is what catches cwd-dependent loading; hitting /health twice is what catches a duplicate request listener. CI previously only listed the generated files and printed package.json, which is why every one of these bugs passed it. Also fills the empty `author` field in package.json and keeps `scripts/` out of the published package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW
- Adds the MIT LICENSE file the README has been advertising with a badge since the first commit. - Removes `src/models/Product.js`, `ProductResolver.js` and `Product.graphql`: generated output accidentally committed into the CLI's own source tree and published to npm with every release. - Removes the empty `src/utils/fs.js` placeholder. - README no longer tells users to run `npm install` after `aapi create`, which now installs for them, and spells out that the secure template requires real JWT secrets. - CHANGELOG records the fixes in this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW
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.
Why
aapi generate model <Name> --relations ...produced a Mongoose model that does not parse:Pulling that thread turned up a chain of defects across the generator, the templates and the security mode. Four out of six
generateflags were broken, generated tests could never run, and the default Yoga server died on its first/healthrequest.None of it was caught because CI only listed the generated files and printed
package.json— it never parsed, installed, ran or started anything it produced.What was broken
Reported first, fixed here
<%= %>--relationsis used--with-subscriptions/--with-cacheimportutils/pubsub.jsandutils/cache.js--no-testsreadoptions.withTestsoptions.tests, so it never had any effect../../models/…fromsrc/__tests__/jest.config.js/jest.setup.jswere never copied and notestscript or Jest dependency was declaredprocess.cwd()--securefalls back to a hardcoded signing key.envsigns tokens with a key published in this repo: anyone can forge an admin tokenFound only by actually booting the generated servers
/healthregistered as a secondrequestlistener alongside YogaERR_HTTP_HEADERS_SENTand kills the process on the first health check — i.e. the first load-balancer probepubsub.jsimportsPubSubfromgraphql-yogacreatePubSub; subscriptions could never workcacheResolverreadscontext.skipCacheunconditionallysetIntervalneverunref()edSmaller
--skip-installskipped nothing (install never ran at all) ·--secure --apollosilently dropped security ·.gitignore/.editorconfigtemplates absent from the npm tarball (npm always strips.gitignore) ·aapi listreported the schema's state on the "Model" line ·aapi --versionhardcoded · Windows-brokennew URL().pathname·apollo-server-express@3EOL since Oct 2023 · strayProductartifacts committed into the CLI's ownsrc/and published to npm · noLICENSEdespite the badge.What changed
--secureserver now refuses to start in production when a JWT secret is missing, still the placeholder, under 32 characters, or identical between access and refresh tokens. Outside production it warns instead of blocking.apollo-server-expressv3 to@apollo/serverv4 +@as-integrations/express4.aapi createnow actually installs dependencies;--skip-installgenuinely opts out.--with-subscriptionsfails with an actionable message on non-Yoga projects rather than emitting dead imports (Apollo Server 4 needsgraphql-wswiring AAPI does not scaffold).Verification
Two layers, both in CI:
__tests__/commands/generate.integration.test.js— scaffolds projects and asserts every generated file parses undernode --check, with targeted guards per regression. Mutation-tested: reverting the<%-fix makes it fail.npm run verify:generated/verify:generated:apollo— scaffold → generate → type-check → install → run the project's own suite → boot the server from/→ hit/healthtwice → CRUD round-trip over GraphQL.Starting outside the project root is what catches cwd-dependent loading; hitting
/healthtwice is what catches the duplicate listener.Results on both server flavours:
Repo itself: lint clean, 59/59 tests.
Reviewing
Seven thematic commits, each self-contained —
git log -p main..HEADreads in order. Two deliberate exceptions to one-theme-per-commit, called out in the commit bodies:init.jscarries the Apollo 4 dependency list in the CLI commit, and the Apollopackage.json.ejscarries itstestscript in the security commit. That beat hunk-level splitting, which does not play well withlint-stagedon partially-staged files.🤖 Generated with Claude Code
https://claude.ai/code/session_01Ao3xH9yfS95UgmBm1VquAW