refactor: rename the example app to what it actually is, a consumer typecheck fixture - #57
Conversation
…ypecheck fixture
`examples/nest-queue-example/` shared its name with `bymaxone/nest-queue-example`,
the real reference application, while being something else entirely: 381 lines that
CI compiles to prove the public API still works in the shape a consumer writes. Two
different things under one name, and the README pointed the "Example App" link at
the wrong one.
Renamed to `test/consumer-app`, package `nest-queue-consumer-app`, CI job "Consumer
app typecheck". The README's Example App link now points at the reference
repository, and the tip block says plainly what the fixture is and is not.
Keeping it rather than deleting it, because it is the only gate that catches an
ergonomic break in the decorator API. Verified by experiment: changing
`@Processor(queueName: string)` to take an options object left `test:types`,
`build` and `smoke` all green and failed only here, with
`TS2345: Argument of type 'string' is not assignable to parameter of type
'{ queue: string }'`. The three gates cross different paths on purpose —
`test:types` compiles `src` through `paths`, `smoke` resolves the packed tarball at
runtime, and this one compiles `dist` through the `exports` map.
That reasoning now lives in the fixture's README and in the CI step comment, since
the next person to see a directory of application code inside a library will
reasonably wonder why it is there.
Paths updated in the planning documents too: they record what was built and the
artifact still exists, so a stale path would just send a reader nowhere.
NOTE: the required status check is renamed. The repository ruleset still requires
"Build & lint example" and must be updated to "Consumer app typecheck", otherwise
every pull request waits forever on a check that no longer reports.
There was a problem hiding this comment.
🟡 Not ready to approve
The technical specification text is now inaccurate about the fixture’s connection setup, and the CI job name change can block merges unless the required-status-check name is handled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Refactors the repository’s “example app” into an explicitly named consumer-shaped typecheck fixture (test/consumer-app) and updates CI/docs to reflect its purpose as an API-ergonomics gate rather than a demo.
Changes:
- Moves/renames the former
examples/nest-queue-exampleintotest/consumer-appwith updated package naming and a fixture-focused README. - Updates CI to build the library and typecheck/lint the consumer app as a dedicated job.
- Sweeps documentation and repo configuration to remove the old “example app” framing and links.
File summaries
| File | Description |
|---|---|
| test/consumer-app/tsconfig.json | Adds a strict, consumer-like TS config for the fixture. |
| test/consumer-app/src/redis.module.ts | Provides a dedicated ioredis client module for Mode A wiring. |
| test/consumer-app/src/main.ts | Adds a bootstrap entrypoint for manual exploration. |
| test/consumer-app/src/health.controller.ts | Adds a /health endpoint consuming Metrics/Queue services. |
| test/consumer-app/src/email.service.ts | Demonstrates enqueueing, Job Schedulers, and flow usage. |
| test/consumer-app/src/email.processor.ts | Demonstrates decorator-based processor + worker event handling. |
| test/consumer-app/src/app.module.ts | Wires BymaxQueueModule.forRootAsync using the dedicated Redis client. |
| test/consumer-app/README.md | Documents the fixture’s purpose and why it exists as a CI gate. |
| test/consumer-app/package.json | Renames the workspace package and updates its description. |
| test/consumer-app/eslint.config.mjs | Adds per-fixture ESLint config for linting. |
| .github/workflows/ci.yml | Renames/retools the job to typecheck/lint the consumer fixture after building the library. |
| README.md | Fixes the “Example App” link to point to the external reference repo. |
| SECURITY.md | Updates scope text to reflect fixtures under test/. |
| pnpm-workspace.yaml | Swaps workspace package inclusion from examples/* to test/consumer-app. |
| eslint.config.mjs | Updates ignore patterns to exclude test/consumer-app/** from root linting. |
| docs/technical_specification.md | Updates spec text to describe the fixture as a PR gate. |
| docs/tasks/phase-05-release.md | Updates planning-doc paths/references from examples/... to test/consumer-app/.... |
| pnpm-lock.yaml | Updates the importer path and bumps locked versions accordingly. |
| examples/nest-queue-example/README.md | Removes the old example README (fixture moved/renamed). |
| examples/nest-queue-example/eslint.config.mjs | Removes the old example ESLint config (fixture moved/renamed). |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 13/20 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Section 19.4 described the fixture as Mode A "with @bymax-one/nest-cache". It has never depended on nest-cache: it wires its own ioredis client through a local RedisModule. Mode A is the shared trait, not the source of the client.
There was a problem hiding this comment.
🟡 Not ready to approve
Some documentation still references the old nest-queue-example fixture name, undermining the rename’s goal.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
docs/technical_specification.md:55
- The PR description says
docs/PHASE_EXECUTION_PROMPT.mdhad its paths updated, but that file still refers to the Phase 5 "nest-queue-exampledogfood" (e.g. arounddocs/PHASE_EXECUTION_PROMPT.md:78). To fully remove the old name/path confusion, that planning doc should be updated to referencetest/consumer-appinstead.
10. **A consumer-shaped typecheck fixture** (`test/consumer-app`) compiles application code against the built package on every pull request.
- Files reviewed: 13/20 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
The fixture was still called `nest-queue-example` in the phase documents. That name now belongs to bymaxone/nest-queue-example, the reference application, so the two artifacts were sharing one name -- the exact confusion this rename set out to remove. The paths were already updated; the name was not. References to the separate repository are left alone: they are correct. Also drops a stray unclosed code fence at the end of PHASE_EXECUTION_PROMPT.md, which left the document with nine fences and an unterminated block. It predates this branch. Closing it lets Prettier format the region it had been skipping, which is the blank-line normalisation in the rest of the diff.
|
Addressed in ffdbe1d — the finding was right, and my PR description was misleading about it. I wrote that the planning documents "had their paths updated". They did. But the fixture was still named Classified rather than bulk-replaced
Six references are left alone because they are correct — they point at the real repository:
One of the renamed lines also carried the same One unrelated thing this surfaced
It matters here for a mechanical reason: Prettier had been skipping everything it believed was inside that block, so I removed the fence rather than let Prettier balance it by appending a second one at EOF, which is what it wanted to do. VerificationEvery surviving |
There was a problem hiding this comment.
🟡 Not ready to approve
There are a couple of concrete fixable issues in the updated files (including a broken Markdown list line and a small maintainability problem in the consumer-app factory signature).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (1)
docs/PHASE_EXECUTION_PROMPT.md:412
- Markdown list formatting is broken here: this continuation line is missing indentation, so it no longer renders as part of the bullet describing BullMQ API rules.
limit, status?)` mirrors `clean(grace, limit, type)`. Sandboxed processors are file-based
- Files reviewed: 14/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
There was a problem hiding this comment.
🟡 Not ready to approve
A few small but concrete issues remain (notably doc/Markdown formatting and an improved factory signature/error message in the fixture) that should be corrected to keep the repo’s documentation and diagnostics consistent with the new “fixture, not demo” framing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
docs/PHASE_EXECUTION_PROMPT.md:412
- Markdown list continuation indentation is broken here: the wrapped
cleanQueue(...)sentence continues on a new line without list indentation, which can render incorrectly. Indent the continuation line to match the surrounding bullet formatting.
- **Current BullMQ API only** — recurring jobs via `upsertJobScheduler`/`removeJobScheduler`/
`getJobSchedulers`; **never** `addRepeatable`/`removeRepeatable` (removed in v6). Cron parsing is
delegated to BullMQ (no `cron-parser` direct dep, no hand-rolled regex). `cleanQueue(queueName, grace,
limit, status?)` mirrors `clean(grace, limit, type)`. Sandboxed processors are file-based
(`registerSandboxed`, no NestJS DI) — there is no `sandboxed: boolean`. The public surface never
docs/tasks/phase-05-release.md:600
- This section was renamed to the
test/consumer-app"dogfood fixture", but the Acceptance criteria immediately below still describes the Redis client as coming from@bymax-one/nest-cacheand calls the harness "demonstration code". Updating those bullets keeps the planning doc consistent with the current fixture setup (localRedisModuleBYO client) and with the "fixture, not demo" framing used elsewhere in this PR.
Build the `test/consumer-app` dogfood fixture that consumes the built surface end-to-end — Mode A with a dedicated `ioredis` client, a `@Processor`, a Job Scheduler, a flow, and a `/health` queue endpoint — and runs in CI before a release is finalized so a contract change that breaks a consumer fails CI.
- Files reviewed: 14/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…ntinuation Task 5.6's acceptance criteria and agent prompt still described the fixture as injecting its client from @bymax-one/nest-cache and called it demonstration code. Neither was ever true of what was built: the client comes from a local RedisModule, and the artifact is a compile-time gate. This is where the claim corrected in the specification originated. The BullMQ bullet in PHASE_EXECUTION_PROMPT.md lost its continuation indent because the cleanQueue(...) code span wrapped across two lines -- leading whitespace inside a span is content, so Prettier strips it and the line falls out of the list. Keeping the span on one line fixes it at the source rather than fighting the formatter.
|
Both addressed in f485588. Both were real, and one of them was mine.
|
There was a problem hiding this comment.
🟡 Not ready to approve
The consumer-app tsconfig currently enables skipLibCheck, which can reduce the fixture’s effectiveness as a consumer-compatibility gate, and there’s a small maintainability issue in the module useFactory signature.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 14/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…ntime skipLibCheck was true, so the fixture never read dist/*.d.ts -- the one thing it is uniquely placed to check. Proven rather than assumed: injecting an import of an uninstalled package into the built declarations reproduces TS2307: Cannot find module 'express' which the fixture caught with skipLibCheck false and passed silently with it true. That is the exact defect nest-logger shipped in 1.0.0 and had to correct in 1.0.4, and no other gate here sees it: test:types compiles src, the smoke test only exercises runtime. Also documents why the useFactory is variadic. The declared parameter type rejects the narrower factory both the README and the module's own @example show, so the narrowing is the fixture reproducing what a consumer must write.
|
Both findings addressed in 59f845a. The first is the strongest review comment this PR has had; the second is real but is not what it looks like.
|
skipLibCheck |
Result |
|---|---|
false |
dist/server/index.d.ts(1,30): error TS2307: Cannot find module 'express' — caught |
true (state of this PR) |
passes silently — blind |
That is not a hypothetical error string. It is the defect @bymax-one/nest-logger shipped in 1.0.0 and had to correct in 1.0.4: the published declarations imported from express, every consumer compiling with skipLibCheck: false got TS2307, and it survived four releases because nothing in that repo read the emitted .d.ts.
This fixture is the only gate here positioned to see it — test:types compiles src through a paths mapping, and pnpm smoke only exercises runtime. Leaving skipLibCheck: true meant the one gate that could catch it had the check switched off.
The fixture compiles clean with it disabled, so the change costs nothing: tsc --noEmit, build and lint all pass.
The useFactory signature — real, but it is a library defect, not fixture untidiness
The awkward shape is not a maintainability slip. It is forced:
useFactory?: (...args: unknown[]) => Promise<BymaxQueueModuleOptions> | BymaxQueueModuleOptionsA parameter typed unknown accepts no narrower parameter under strictFunctionTypes, so the form the README and the module's own @example both show does not compile:
TS2322: Type '(client: Redis) => { connection: { client: Redis } }' is not assignable
to type '(...args: unknown[]) => BymaxQueueModuleOptions | Promise<…>'.
Types of parameters 'client' and 'args' are incompatible.
Type 'unknown' is not assignable to type 'Redis'.
Reproduced against the declared interface, not inferred. It affects README.md:242 and bymax-queue.module.ts:58 — every consumer who copies the documented snippet gets a type error, on a package that is already published at 1.0.2.
So the fixture is behaving correctly: it reproduces what a consumer must actually write, and that ugliness is the finding. Tidying it away would have deleted the evidence — which is precisely the failure mode this fixture exists to prevent.
What I did instead: kept the shape, sharpened the error message you flagged, and documented why it is variadic so the next reader does not "clean it up".
What I did not do: change the declared parameter type. CLAUDE.md freezes the public surface pending a deliberate versioned decision, and widening it is a library change that does not belong in a PR about renaming a directory. It needs its own PR, its own changelog entry, and a test/types/public-api.test-d.ts case — which currently pins nothing for useFactory, and is why this went unnoticed.
Raised with the maintainer as a separate decision.
Gate
tsc --noEmit (fixture, skipLibCheck: false) · build · lint · library typecheck · test:types · lint — all green before push.
{
"pr": 57,
"localGate": "pnpm typecheck && pnpm test:types && pnpm lint (+ fixture tsc/build/lint)",
"consecutiveFailures": {},
"flakyReruns": {},
"processedCommentIds": [3686972728, 3686972762],
"processedReviewFindings": [
"2026-07-30T23:36:05Z ruleset-rename + spec nest-cache claim",
"2026-08-01T11:08:17Z stale nest-queue-example name in planning docs",
"2026-08-01T12:29:34Z + 12:39:49Z broken list continuation + acceptance criteria",
"2026-08-01T12:49:25Z skipLibCheck + useFactory signature"
],
"openDecisionForMaintainer": "useFactory declared as (...args: unknown[]) rejects the factory shape documented in README.md:242 and bymax-queue.module.ts:58 — public-surface change, needs its own PR",
"paused": false,
"note": "Copilot delivers findings in the review BODY as suppressed comments, not as review threads. A thread count of 0 does NOT mean no outstanding findings — always read the newest review body."
} |
There was a problem hiding this comment.
🟢 Ready to approve
The changes consistently complete the rename/relocation and update CI + documentation accordingly, with no correctness issues found in the updated fixture wiring.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 16/21 changed files
- Comments generated: 0 new
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
examples/nest-queue-example/shared its name withbymaxone/nest-queue-example, the real reference application — while being something else entirely. That is what made you ask what the folder was for, and the confusion was earned: the README pointed its "Example App" link at the wrong one of the two.What it actually is
381 lines that CI compiles to prove the public API still works in the shape a consumer writes. It is a gate, not a demo. Renamed accordingly:
examples/nest-queue-example/test/consumer-app/nest-queue-examplenest-queue-consumer-appBuild & lint exampleConsumer app typecheckbymaxone/nest-queue-exampleWhy it is kept rather than deleted
It is the only gate that catches an ergonomic break in the decorator API — and I checked rather than assumed. Changing
@Processor(queueName: string)to take an options object:pnpm test:typessrc/viapathspnpm buildpnpm smokedist/viaexportsTS2345: Argument of type 'string' is not assignable to parameter of type '{ queue: string }'The three cross different paths on purpose. This library's surface is decorator-heavy —
@Processor,@Process,@OnWorkerEvent,@OnQueueEvent— and a decorator's ergonomics can break without any exported signature changing shape. The sibling libs have zero or one decorator file, which is why none of them needs this and none of them has it.That reasoning now lives in the fixture's README and in the CI step comment, since the next person to find a directory of application code inside a library will reasonably wonder why.
Documentation swept
README.md(nav link + the tip block, which now says plainly what the fixture is and is not) ·SECURITY.mdscope ·docs/technical_specification.md§0 and the dogfood section ·pnpm-workspace.yaml·eslint.config.mjs·ci.yml· the fixture's own README, rewritten.The planning documents (
docs/tasks/phase-05-release.md,docs/PHASE_EXECUTION_PROMPT.md) keep their narrative — they record what was built — but their paths are updated, because the artifact still exists and a stale path just sends a reader nowhere.The required status check is renamed.
nest-queue-pr-and-ci-gatesstill requires "Build & lint example", which no longer reports — so this PR, and every PR after it, waits forever unless the ruleset is updated to "Consumer app typecheck".This PR already produces the new check, and there are no other open PRs, so the swap is safe to do directly. Say the word and I will make it; I have the current ruleset backed up.
Verification
typecheck · test:types · lint · 276 tests at 100% · build · size · check:exports (12/12) · dogfood smoke · fixture typecheck + lint.
Ready for your review — not merging it.