Gh 11 deploy to ecs#16
Conversation
WalkthroughThis change is a large-scale, multi-repository refactor and configuration update focused on renaming the package namespace from Changes
Changes Table (Compact)
Sequence Diagram(s)sequenceDiagram
participant Developer
participant GitHub Registry
participant Workspace (pnpm/npm)
participant Docker Build
participant App
Developer->>Workspace: Update package.json to @hipponot/* scope
Developer->>Workspace: Install dependencies (pnpm/npm)
Workspace->>GitHub Registry: Fetch @hipponot/* packages (public, no auth needed)
Developer->>Docker Build: Build image with .npmrc for @hipponot/* registry
Docker Build->>GitHub Registry: Fetch @hipponot/* packages (with NPM_TOKEN for private publish)
Docker Build->>App: Build and run app with installed packages
Estimated code review effort5 (~2–4 hours) This change is extensive, affecting a large number of files across multiple packages and documentation, with high-complexity refactoring, configuration, and documentation updates. Reviewers should allocate significant time for careful validation of package renaming, dependency and import correctness, build/publish workflow integrity, and documentation accuracy. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (6)
packages/core-api/src/express-server.ts (1)
26-28: Filtering controllers byinitdrops legitimate controllers—remove the filterControllers without an
initmethod are silently excluded, meaning their routes never get registered.
Keep all supplied controllers, and conditionally callinitlater:- const controllerClasses = [...controllers, SectorsController].filter( - (ctrl): ctrl is new (...args: any[]) => any => typeof ctrl === 'function' && ctrl.prototype && ctrl.prototype.init - ); + const controllerClasses = [...controllers, SectorsController];You already guard the
initinvocation withif (typeof controllerInstance.init === 'function'), so the filter is unnecessary.packages/eslint-config/package.json (1)
19-30:typescript-eslintpackage is invalid – replace with the official scoped plugins
There is no package published to npm under the bare nametypescript-eslint.
ESLint integrations are exposed as@typescript-eslint/eslint-plugin(rules) and@typescript-eslint/parser(parser). Keeping the current entry will breakpnpm install."devDependencies": { @@ - "typescript-eslint": "^8.33.0" + "@typescript-eslint/eslint-plugin": "^8.33.0", + "@typescript-eslint/parser": "^8.33.0" }packages/core-api/package.json (1)
46-52:vitestscript without dependency
"test": "vitest run"remains, butvitestwas removed fromdevDependencies;pnpm testwill fail.+ "vitest": "^1.5.2"or remove the script.
human-notes/current-chat.md (3)
1452-1460:PinoLoggerSchematyping is too generic – Rely on Zod’s shape instead ofZodObject<HasConfigType>
ZodObject<T>expectsTto be a “raw shape” (i.e.{ [k:string]: ZodType }).
HasConfigTypeis almost certainly declared as an interface / type alias containing TypeScript‐only members, not a Zod raw shape; passing it here defeats Zod’s compile-time checks and widens the acceptable object.-export const PinoLoggerSchema: ZodObject<HasConfigType> = z.object({ +export const PinoLoggerSchema = z.object({Then export the inferred type as you already do:
export type PinoLoggerConfig = z.infer<typeof PinoLoggerSchema>;
1038-1051: Transport target'pino/file'is not a built-in transport
pino≥ 8 ships only the'pino-pretty'transport. File logging is done by piping stdout to a file or by using community transports (e.g.pino/file,pino/file-transport). The current string will silently fail at runtime.Verify the intended transport or switch to the recommended pattern:
- targets.push({ - target: 'pino/file', - options: { destination: logFile }, - }); + targets.push({ + target: 'pino/file-transport', + options: { destination: logFile, mkdir: true }, + });(or use
pino.destination(logFile)when not in multi-transport mode).
2229-2234: Route registration duplicates segment – prefer a child router
router.get(\${sectorBase}/hello`)yields/saga-soa/hello/hello`, unintuitive for consumers. Mount a child router instead:public registerRoutes(router: Router): void { - const sectorBase = `${this.apiBaseUrl}/${this.sectorName}`; - router.get(`${sectorBase}/alive`, this.aliveRoute()); - router.get(sectorBase, this.sectorHomeRoute()); - this.registerSectorRoutes(router, sectorBase); + const sectorBase = `${this.apiBaseUrl}/${this.sectorName}`; + const sectorRouter = Router(); + sectorRouter.get('/', this.sectorHomeRoute()); + sectorRouter.get('/alive', this.aliveRoute()); + this.registerSectorRoutes(sectorRouter, '/'); + router.use(sectorBase, sectorRouter); }Sector implementations can now add
router.get('/hello', …)without repeating the base path.
🧹 Nitpick comments (27)
packages/logger/memory-bank/logger.md (1)
1-11: Header updated, but body still references the old scopeLines 3 & 7 still say “saga-soa”. For consistency (and to avoid copy-paste confusion) update those occurrences to
hipponot.-This package provides a configurable logging solution for the saga-soa project. +This package provides a configurable logging solution for the hipponot project. @@ -- **Pino-based**: Leverages the speed and structured logging capabilities of Pino. Using ping-http to adapt to use in an express context. +- **Pino-based**: Leverages the speed and structured logging capabilities of Pino. Using ping-http to adapt to use in an express context..npmrc (1)
6-10: Re-evaluatelegacy-peer-deps&auto-install-peers—can silently mask version conflictsTurning on legacy peer-dependency resolution together with automatic peer installs trades reliability for convenience: npm will stop warning on incompatible versions and will pull in whatever is “closest”, which can lead to hard-to-debug runtime breakages.
Consider enabling these flags only in CI / publish pipelines (where reproducible lockfiles are produced) and keep them disabled for local installs, or document the exact rationale so future maintainers understand the risk.packages/db/src/__tests__/mongo-provider.test.ts (1)
5-6: Avoid deep internal imports—export mocks from package root
@hipponot/config/mocks/mock-config-manager.jscouples the test to the package’s internal layout; any refactor will break consumers.
ExposeMockConfigManagervia the mainindex.tsof@hipponot/config(or a dedicated testing export) and import simply as:import { MockConfigManager } from '@hipponot/config/testing';memory-bank/type-resolution-checklist.md (2)
3-3: Update monorepo name to avoid confusion.The intro still says “saga-soa monorepo”; this is out of sync with the new
@hipponotbranding.-...in the saga-soa monorepo. +...in the hipponot monorepo.
25-28: Consider keeping the workspace protocol for internal deps.Hard-coding
@hipponot/soa-logger@1.0.0loses the automatic linking you get with:pnpm add @hipponot/soa-logger@workspace:* --filter <app-path>Unless you plan on publishing and consuming a fixed version, the workspace protocol remains clearer for local development.
packages/core-api/memory-bank/express-server.md (1)
3-3: Align branding terminology throughout the doc
You switched the namespace to@hipponot, but the intro sentence still talks about the “saga-soa monorepo” in several places above. For clarity, update the remaining references so readers aren’t left wondering whether both scopes are still active.docs/library-guide.md (2)
8-8: Update monorepo name to match new scope
Line 3 still says “saga-soa monorepo” while the example package name has moved to@hipponot. Pick one naming convention to avoid confusion.
43-43: Prefer workspace protocol over hard-coding “1.0.0”
Pinning internal deps to1.0.0means every bump requires manual doc edits and breaks local linking during active development. Consider:- "@hipponot/soa-config": "1.0.0" + "@hipponot/soa-config": "workspace:*"This keeps the example aligned with the rest of the monorepo tooling.
packages/core-api/src/sectors-controller.ts (1)
68-68: Remove or implement the no-opinit()method
An emptyasync init() {}is dead weight and suggests unfinished work. Either drop it or implement the intended startup logic.docs/express-api-guide.md (2)
8-8: Consistent monorepo naming
Similar to the library guide, the intro still refers to the “saga-soa monorepo” while the example uses@hipponot. Harmonise the wording to avoid mixed branding.
41-42: Avoid hard-coding internal package versions in examples
Useworkspace:*(or leave the version out) so copy-pastedpackage.jsons don’t become stale the moment the next release is cut.packages/ui/package.json (1)
15-16: Consider workspace ranges instead of hard-pinning internal devDependenciesPinning to
0.0.1forces every bump to flow through all packages manually and preventspnpm --filter ...workspace linking.
Using"workspace:*"or at least a caret (^0.0.1) keeps version drift lower-friction.README.md (2)
24-32: Project branding is half-updatedLines 24–32 correctly introduce the
@hipponotscope, but the top heading (# saga-soa) and several blocks above still say saga-soa.
Search/replace the remaining occurrences to avoid confusing newcomers.
38-49: Scope-only.npmrcentry is saferAppending
@hipponot:registry=https://npm.pkg.github.comto the global
.npmrcaffects all projects on the machine.
Recommend a per-project.npmrcor:npm config set @hipponot:registry https://npm.pkg.github.com --location projectto localise the change.
packages/logger/package.json (1)
17-18: Use cross-platform cleaner instead ofrm -rf
rmbreaks on Windows shells. Swap forrimraf(already a dep oftsup) or nativepnpm dlx rimraf.- "clean": "rm -rf dist", + "clean": "rimraf dist",turbo.json (1)
35-44:pre-publishtask has no outputs – disable caching explicitlyWith an empty
outputsarray Turbo still caches the task.
Either add an output (e.g.dist/**) or mark the task non-cacheable:- "pre-publish": { + "pre-publish": { + "cache": false,packages/typescript-config/package.json (1)
13-17: Consider adding afileswhitelist to reduce publish size
Right now the entire folder (including tests, docs, etc.) will be shipped.
Adding"files": ["base.json", "react-library.json", "nextjs.json"]keeps the package footprint minimal.
package.json (1)
7-26: Script naming & recursion risk
pre-publishis not an npm lifecycle hook (prepublishOnlyis).
Invoking it manually is fine, but the name is misleading.publish:packagesrunspnpm pre-publish, which in turn runspnpm build:clean && pnpm test.
If a contributor later renames the helper script toprepublishOnly, the chain will silently break.Recommend:
- "pre-publish": "pnpm build:clean && pnpm test", + "prepublishOnly": "pnpm build:clean && pnpm test", ... - "publish:packages": "pnpm pre-publish && pnpm -r publish --access public", + "publish:packages": "pnpm run -w --if-present prepublishOnly && pnpm -r publish --access public",packages/db/package.json (1)
18-24:rm -rfis not cross-platform
Windows contributors (or CI images) without a POSIX shell will fail on thecleanscript.- "clean": "rm -rf dist", + "clean": "rimraf dist",Add
rimraftodevDependenciesif not already present.docs/migration-checklist.md (1)
78-90: Minor wording: replace “NPM_TOKEN build argument” with “NPM_TOKENbuild-arg” for clarity
Pure documentation, no functional impact – feel free to ignore.docs/GETTING-STARTED.md (2)
19-20: Update branding referenceStill refers to “saga-soa monorepo”; should now be hipponot for consistency with the repo–wide rename.
175-176: Minor grammar nit – compound adjectiveUse a hyphen in “backwards-compatible”.
- New features, backwards compatible + New features, backwards-compatibledocs/npm-registry-publishing.md (1)
131-135: Hyphenate “backwards-compatible”Same grammar nit as above for consistency throughout docs.
packages/core-api/package.json (2)
50-51: Cross-platform clean
rm -rf distbreaks on Windows. Preferrimraf.- "clean": "rm -rf dist", + "clean": "rimraf dist", + "prepare": "pnpm build"
54-55: Internal version skewDepends on
@hipponot/soa-logger0.0.1while this package is already0.0.2. If they are released together, bump for consistency.human-notes/current-chat.md (2)
1023-1046: Avoid magic-string injection tokens – declare explicit SymbolsHard-coding
"PinoLoggerConfig"/"ILogger"degrades type-safety and makes container re-use brittle. Prefer a centralTYPESmap:-@inject('PinoLoggerConfig') private config: PinoLoggerConfig +@inject(TYPES.PinoLoggerConfig) private config: PinoLoggerConfigThe same applies to the
ILoggerbinding inExpressServer. Provide:export const TYPES = { ILogger: Symbol.for('ILogger'), PinoLoggerConfig: Symbol.for('PinoLoggerConfig'), // … } as const;
2276-2280:/hello/helloendpoint caused by duplicated pathIf the previous comment is not adopted, at least fix the immediate duplication:
- router.get(`${sectorBase}/hello`, this.helloRoute()); + router.get(`${sectorBase}`, this.helloRoute());…but the child-router refactor is the cleaner long-term solution.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (21)
apps/docs/app/favicon.icois excluded by!**/*.icoapps/docs/app/fonts/GeistMonoVF.woffis excluded by!**/*.woffapps/docs/app/fonts/GeistVF.woffis excluded by!**/*.woffapps/docs/public/file-text.svgis excluded by!**/*.svgapps/docs/public/globe.svgis excluded by!**/*.svgapps/docs/public/next.svgis excluded by!**/*.svgapps/docs/public/turborepo-dark.svgis excluded by!**/*.svgapps/docs/public/turborepo-light.svgis excluded by!**/*.svgapps/docs/public/vercel.svgis excluded by!**/*.svgapps/docs/public/window.svgis excluded by!**/*.svgapps/web/app/favicon.icois excluded by!**/*.icoapps/web/app/fonts/GeistMonoVF.woffis excluded by!**/*.woffapps/web/app/fonts/GeistVF.woffis excluded by!**/*.woffapps/web/public/file-text.svgis excluded by!**/*.svgapps/web/public/globe.svgis excluded by!**/*.svgapps/web/public/next.svgis excluded by!**/*.svgapps/web/public/turborepo-dark.svgis excluded by!**/*.svgapps/web/public/turborepo-light.svgis excluded by!**/*.svgapps/web/public/vercel.svgis excluded by!**/*.svgapps/web/public/window.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (107)
.cursor/rules/naming-conventions.mdc(1 hunks).npmrc(1 hunks)README.md(1 hunks)apps/docs/.gitignore(0 hunks)apps/docs/README.md(0 hunks)apps/docs/app/globals.css(0 hunks)apps/docs/app/layout.d.ts(0 hunks)apps/docs/app/layout.js(0 hunks)apps/docs/app/layout.tsx(0 hunks)apps/docs/app/page.d.ts(0 hunks)apps/docs/app/page.js(0 hunks)apps/docs/app/page.module.css(0 hunks)apps/docs/app/page.tsx(0 hunks)apps/docs/eslint.config.js(0 hunks)apps/docs/next.config.js(0 hunks)apps/docs/package.json(0 hunks)apps/docs/tsconfig.json(0 hunks)apps/examples/graphql-api/memory-bank/schema-design-plan.md(0 hunks)apps/examples/graphql-api/package.json(0 hunks)apps/examples/graphql-api/scripts/emit-schema.ts(0 hunks)apps/examples/graphql-api/src/inversify.config.ts(0 hunks)apps/examples/graphql-api/src/main.ts(0 hunks)apps/examples/graphql-api/src/sectors/index.ts(0 hunks)apps/examples/graphql-api/src/sectors/session/gql/index.ts(0 hunks)apps/examples/graphql-api/src/sectors/session/gql/session.data.ts(0 hunks)apps/examples/graphql-api/src/sectors/session/gql/session.input.ts(0 hunks)apps/examples/graphql-api/src/sectors/session/gql/session.resolver.ts(0 hunks)apps/examples/graphql-api/src/sectors/session/gql/session.type.ts(0 hunks)apps/examples/graphql-api/src/sectors/session/index.ts(0 hunks)apps/examples/graphql-api/src/sectors/session/rest/session-routes.ts(0 hunks)apps/examples/graphql-api/src/sectors/user/gql/index.ts(0 hunks)apps/examples/graphql-api/src/sectors/user/gql/user.data.ts(0 hunks)apps/examples/graphql-api/src/sectors/user/gql/user.input.ts(0 hunks)apps/examples/graphql-api/src/sectors/user/gql/user.resolver.ts(0 hunks)apps/examples/graphql-api/src/sectors/user/gql/user.type.ts(0 hunks)apps/examples/graphql-api/src/sectors/user/index.ts(0 hunks)apps/examples/graphql-api/src/sectors/user/rest/user-routes.ts(0 hunks)apps/examples/graphql-api/tsconfig.json(0 hunks)apps/examples/graphql-api/tsup.config.ts(0 hunks)apps/examples/graphql-api/vitest.config.ts(0 hunks)apps/examples/rest-api/.vscode/launch.json(0 hunks)apps/examples/rest-api/package.json(0 hunks)apps/examples/rest-api/src/__tests__/integration.test.ts(0 hunks)apps/examples/rest-api/src/inversify.config.ts(0 hunks)apps/examples/rest-api/src/main.ts(0 hunks)apps/examples/rest-api/src/sectors/hello-again-rest.ts(0 hunks)apps/examples/rest-api/src/sectors/hello-mongo.ts(0 hunks)apps/examples/rest-api/src/sectors/hello-rest.ts(0 hunks)apps/examples/rest-api/tsconfig.json(0 hunks)apps/examples/rest-api/tsup.config.ts(0 hunks)apps/examples/rest-api/vitest.config.ts(0 hunks)apps/web/.gitignore(0 hunks)apps/web/README.md(0 hunks)apps/web/app/globals.css(0 hunks)apps/web/app/layout.d.ts(0 hunks)apps/web/app/layout.js(0 hunks)apps/web/app/layout.tsx(0 hunks)apps/web/app/page.d.ts(0 hunks)apps/web/app/page.js(0 hunks)apps/web/app/page.module.css(0 hunks)apps/web/app/page.tsx(0 hunks)apps/web/eslint.config.js(0 hunks)apps/web/next.config.js(0 hunks)apps/web/package.json(0 hunks)apps/web/tsconfig.json(0 hunks)docs/GETTING-STARTED.md(5 hunks)docs/docker-registry-examples.md(1 hunks)docs/express-api-guide.md(2 hunks)docs/library-guide.md(2 hunks)docs/migration-checklist.md(1 hunks)docs/npm-registry-publishing.md(1 hunks)human-notes/current-chat.md(64 hunks)human-notes/turborepo-notes.md(2 hunks)memory-bank/naming-conventions.md(1 hunks)memory-bank/testing/testing-improvement-plan.md(1 hunks)memory-bank/turborepo.md(2 hunks)memory-bank/type-resolution-checklist.md(2 hunks)memory-bank/unit-testing.md(2 hunks)package.json(3 hunks)packages/config/README.md(4 hunks)packages/config/package.json(2 hunks)packages/core-api/memory-bank/express-server.md(2 hunks)packages/core-api/package.json(4 hunks)packages/core-api/src/__tests__/express-server.test.ts(1 hunks)packages/core-api/src/express-server.ts(1 hunks)packages/core-api/src/gql-controller.ts(1 hunks)packages/core-api/src/index.ts(1 hunks)packages/core-api/src/rest-controller.ts(1 hunks)packages/core-api/src/sectors-controller.ts(2 hunks)packages/core-api/tsconfig.json(1 hunks)packages/core-api/tsup.config.ts(1 hunks)packages/db/jest.config.cjs(1 hunks)packages/db/package.json(2 hunks)packages/db/src/__tests__/mongo-provider.test.ts(1 hunks)packages/db/src/mocks/mock-mongo-provider.ts(1 hunks)packages/eslint-config/package.json(2 hunks)packages/logger/memory-bank/logger.md(1 hunks)packages/logger/package.json(3 hunks)packages/logger/src/pino-logger.ts(6 hunks)packages/logger/tsconfig.json(1 hunks)packages/typescript-config/package.json(1 hunks)packages/ui/eslint.config.mjs(1 hunks)packages/ui/package.json(3 hunks)packages/ui/tsconfig.json(1 hunks)pnpm-workspace.yaml(1 hunks)tsconfig.json(1 hunks)turbo.json(2 hunks)
🪛 markdownlint-cli2 (0.17.2)
human-notes/turborepo-notes.md
3-3: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/npm-registry-publishing.md
[uncategorized] ~133-~133: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ixes - Minor (1.1.0): New features, backwards compatible - Major (2.0.0): Breaking changes ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
human-notes/current-chat.md
[style] ~1743-~1743: Consider using polite language here.
Context: ...est cases or want to extend the logger, let me know! --- User please run the tests ...
(INSERT_PLEASE)
💤 Files with no reviewable changes (62)
- apps/docs/app/page.d.ts
- apps/docs/next.config.js
- apps/examples/graphql-api/tsconfig.json
- apps/examples/rest-api/tsconfig.json
- apps/examples/rest-api/.vscode/launch.json
- apps/docs/README.md
- apps/web/next.config.js
- apps/examples/graphql-api/src/sectors/session/index.ts
- apps/docs/eslint.config.js
- apps/web/README.md
- apps/docs/.gitignore
- apps/examples/rest-api/tsup.config.ts
- apps/examples/graphql-api/src/sectors/user/index.ts
- apps/examples/graphql-api/package.json
- apps/docs/app/globals.css
- apps/web/eslint.config.js
- apps/docs/tsconfig.json
- apps/web/app/page.d.ts
- apps/examples/graphql-api/src/sectors/index.ts
- apps/examples/graphql-api/src/sectors/user/gql/user.input.ts
- apps/docs/app/layout.tsx
- apps/docs/app/layout.d.ts
- apps/examples/graphql-api/src/sectors/user/gql/index.ts
- apps/examples/graphql-api/tsup.config.ts
- apps/docs/app/layout.js
- apps/examples/rest-api/package.json
- apps/examples/graphql-api/scripts/emit-schema.ts
- apps/examples/graphql-api/src/sectors/session/gql/session.input.ts
- apps/examples/graphql-api/src/sectors/user/rest/user-routes.ts
- apps/examples/graphql-api/vitest.config.ts
- apps/web/app/layout.js
- apps/examples/rest-api/vitest.config.ts
- apps/web/package.json
- apps/docs/app/page.tsx
- apps/examples/rest-api/src/sectors/hello-rest.ts
- apps/web/.gitignore
- apps/examples/rest-api/src/inversify.config.ts
- apps/examples/rest-api/src/sectors/hello-again-rest.ts
- apps/examples/graphql-api/src/sectors/session/gql/index.ts
- apps/examples/graphql-api/src/sectors/session/gql/session.type.ts
- apps/docs/app/page.module.css
- apps/web/app/layout.d.ts
- apps/examples/graphql-api/src/sectors/session/rest/session-routes.ts
- apps/examples/rest-api/src/main.ts
- apps/docs/package.json
- apps/web/tsconfig.json
- apps/web/app/page.module.css
- apps/web/app/page.js
- apps/web/app/layout.tsx
- apps/examples/graphql-api/src/sectors/user/gql/user.resolver.ts
- apps/examples/graphql-api/src/inversify.config.ts
- apps/examples/graphql-api/src/sectors/session/gql/session.resolver.ts
- apps/examples/rest-api/src/tests/integration.test.ts
- apps/examples/graphql-api/src/sectors/user/gql/user.data.ts
- apps/examples/graphql-api/src/sectors/session/gql/session.data.ts
- apps/web/app/page.tsx
- apps/examples/graphql-api/src/sectors/user/gql/user.type.ts
- apps/examples/graphql-api/memory-bank/schema-design-plan.md
- apps/examples/rest-api/src/sectors/hello-mongo.ts
- apps/web/app/globals.css
- apps/examples/graphql-api/src/main.ts
- apps/docs/app/page.js
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
human-notes/turborepo-notes.md
3-3: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
docs/npm-registry-publishing.md
[uncategorized] ~133-~133: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ixes - Minor (1.1.0): New features, backwards compatible - Major (2.0.0): Breaking changes ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
human-notes/current-chat.md
[style] ~1743-~1743: Consider using polite language here.
Context: ...est cases or want to extend the logger, let me know! --- User please run the tests ...
(INSERT_PLEASE)
🔇 Additional comments (26)
packages/logger/tsconfig.json (1)
2-2: Ensure dev-dependency on the new base config
"extends": "@hipponot/soa-typescript-config/base.json"implies this package now depends on@hipponot/soa-typescript-config.
Double-checkpackages/logger/package.jsonincludes it (usually as a devDependency) to avoid CI failures.packages/ui/tsconfig.json (1)
2-2: Same dependency check applies to@hipponot/soa-typescript-configConfirm
packages/ui/package.jsonlists the matching version; otherwisetsc -bwill fail when the workspace is bootstrapped.packages/config/README.md (1)
1-40: 👍 Namespace rename looks correct hereHeader and code samples have been updated to
@hipponot/*. No further issues spotted in the changed lines.Also applies to: 106-110, 128-132
.npmrc (1)
12-13:enable-pre-post-scripts=trueraises supply-chain attack surfaceAllowing pre/post-install scripts means every transitive dependency may execute arbitrary code on install.
If you don’t actively rely on those hooks (e.g. pnpm lifecycle), setignore-scripts=trueinstead or gate the flag behind an environment variable for trusted CI only.packages/core-api/src/__tests__/express-server.test.ts (1)
5-5: EnsureILoggeris bound in the DI containerI couldn’t locate any explicit
container.bind('ILogger')in your code. Without it, all@inject('ILogger')usages will throw at runtime. Please verify that your DI setup (e.g., ininversify.config.tsor where you bootstrap the container inexpress-server.ts) includes something like:container .bind<ILogger>('ILogger') .to(ConsoleLogger) .inSingletonScope();Key injection sites to double-check against your bindings:
- packages/core-api/src/sectors-controller.ts:14
- packages/core-api/src/gql-controller.ts:9
- packages/core-api/src/express-server.ts:16
packages/core-api/src/gql-controller.ts (1)
1-11: Pure namespace switch—no further action needed
The import path change is correct and nothing else is affected.packages/core-api/src/rest-controller.ts (2)
2-5: Import path update aligns with namespace refactoring.The reformatting and namespace change from
@saga-soa/loggerto@hipponot/soa-loggeris consistent with the broader package renaming effort.
7-7: No change needed for REST_API_BASE_PATHThe
REST_API_BASE_PATHvalue of'saga-soa'aligns with the project’s package name (package.json remains"name": "saga-soa"), and there are no other HTTP routes using an@hipponotprefix—@hipponot/soa-loggeris merely an external logger package import. Keeping the base path as'saga-soa'is intentional for consistency and backward compatibility.memory-bank/unit-testing.md (1)
25-25: Documentation correctly updated for new Jest config package namespace.The references to Jest configuration have been consistently updated from
@saga-soa/jest-configto@hipponot/jest-config, aligning with the broader package namespace migration.Also applies to: 43-43
packages/core-api/tsup.config.ts (1)
5-5: Entry point exists and exports confirmed — configuration approved.
- packages/core-api/src/index.ts is present and exports the intended controllers and utilities
- package.json’s “main”, “types”, and “exports” fields correctly point to the built files under dist/
- Build configuration can safely include
'src/index.ts'as an entry pointpackages/core-api/tsconfig.json (1)
2-2: Make sure the new base config is installed in this workspace.
"extends": "@hipponot/soa-typescript-config/base.json"assumes that@hipponot/soa-typescript-configis listed in this package’sdevDependencies(or is provided via the root).
Please confirm it is present; otherwisetscwill fail to resolve the path.packages/db/jest.config.cjs (1)
5-10: Namespace switch looks correct – no issues detected.The mapper and
transformIgnorePatternshave been updated consistently to the new@hipponotscope; this should keepts-jestresolution intact. No further action needed.memory-bank/naming-conventions.md (1)
147-155: Documentation update is consistent with the new scope.Examples now reference
@hipponot/*; syntax and fenced-block language tags (typescript) are intact. 👍packages/core-api/memory-bank/express-server.md (1)
40-40: Double-check that@hipponot/configis published & version-pinned
The test plan depends on a mock from@hipponot/config. Make sure the new package has actually been published (or is available via workspace protocol) before others try to follow this guide.packages/core-api/src/sectors-controller.ts (1)
2-5: Ensure DI binding matches new logger import
The class injects using the identifier'ILogger'. After migrating to@hipponot/soa-logger, confirm your container still binds that exact token, otherwise runtime resolution will fail.memory-bank/testing/testing-improvement-plan.md (1)
3-3: Mixed scopes may confuse readers
Section headers now list@saga-soa/dbalongside several@hipponot/*packages. Is the DB package intentionally staying under the old scope? If not, update for consistency; if yes, add a note explaining the split branding.docs/express-api-guide.md (1)
47-47: Highlight path alias changes
Nice catch updating the import to the new scope. 👍memory-bank/turborepo.md (1)
4-46: LGTM! Consistent namespace update throughout documentation.The documentation has been systematically updated to reflect the new
@hipponotpackage scope, maintaining consistency across all examples, dependency declarations, and import statements. The technical content and best practices remain intact.packages/logger/src/pino-logger.ts (1)
30-33: LGTM! Consistent formatting improvements.The formatting changes to the pretty print options improve code consistency and readability.
Also applies to: 51-54, 78-81, 93-96
packages/config/package.json (3)
2-12: LGTM! Proper GitHub Packages configuration.The package has been correctly renamed to
@hipponot/soa-configwith appropriate publishing configuration for GitHub Packages. The repository field and publishConfig are properly set up.
18-18: Good practice: Build before publish.Adding
prepublishOnlyscript ensures the package is built before publishing, preventing accidental publication of stale builds.
35-35: Consistent dependency update.The devDependency has been correctly updated to reference the new
@hipponot/soa-typescript-configpackage with the appropriate version.packages/core-api/src/index.ts (1)
3-6: Public-API expansion looks solidNew named exports expose the REST stack and util loader cleanly; paths align with build output. No issues spotted.
packages/eslint-config/package.json (1)
2-12: Confirm package / repository naming consistency
The package is now scoped as@hipponot/soa-eslint-config, but the repository URL still points tohipponot/saga-soa.git.
If the repo itself is going to be renamed tosoa, update the URL to avoid 404s after the rename; otherwise nothing to do.docs/docker-registry-examples.md (1)
44-51: Credential leakage risk in single-stage exampleWriting the token to
~/.npmrcin the same stage bakes the secret into image layers.
Either:
- Use the multi-stage pattern shown later (preferred), or
rm ~/.npmrcin the final stage.Document this explicitly to avoid accidental leaks.
packages/core-api/package.json (1)
7-10:publishConfig.accessmay be ignored by GitHub PackagesGitHub’s npm registry does not honor
"access": "public"; leaving it can causenpm publishto error with “invalid access”.
Confirm publishing succeeds or drop the key.
| - Generic type parameters | ||
| - Decorator naming | ||
| import { add } from @saga/math/add | ||
| import { add } from @saga-soa/math/add |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Inconsistent namespace migration pattern needs verification.
The file shows mixed namespace changes:
- Line 15:
@saga/math/add→@saga-soa/math/add - Line 20:
@saga-soa/config→@hipponot/config
This inconsistency suggests different migration strategies for different packages. Please verify the correct target namespaces to ensure consistency across the codebase.
Also applies to: 20-20
🏁 Script executed:
#!/bin/bash
# Description: Check for namespace consistency across the codebase
# Expected: Identify which packages use which namespaces
echo "Searching for @saga/ package references:"
rg -A 1 "@saga/" --type md --type ts --type js
echo -e "\nSearching for @saga-soa/ package references:"
rg -A 1 "@saga-soa/" --type md --type ts --type js
echo -e "\nSearching for @hipponot/ package references:"
rg -A 1 "@hipponot/" --type md --type ts --type js
echo -e "\nChecking package.json files for namespace patterns:"
fd package.json --exec cat {} \; --exec echo "--- {} ---" \; | grep -A 5 -B 5 '"name":'Length of output: 16956
🏁 Script executed:
#!/bin/bash
# Display the first 200 lines of the naming-conventions rule file for context
cat .cursor/rules/naming-conventions.mdc | sed -n '1,200p'Length of output: 974
Consistently use the @hipponot/soa- scope in all import examples
Please update the example imports in .cursor/rules/naming-conventions.mdc to match the actual package namespace:
• In .cursor/rules/naming-conventions.mdc
– Line 15:
Change
import { add } from @saga-soa/math/add
To
import { add } from '@hipponot/soa-math/add'
– Line 20:
Change
import { IConfigManager, MockConfigManager } from '@hipponot/config';
To
import { IConfigManager, MockConfigManager } from '@hipponot/soa-config';
Diff snippet:
import { MockConfigManager } from './mock-config-manager';
-import { add } from @saga-soa/math/add
+import { add } from '@hipponot/soa-math/add'
// … other items …
-import { IConfigManager, MockConfigManager } from '@hipponot/config';
+import { IConfigManager, MockConfigManager } from '@hipponot/soa-config';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { add } from @saga-soa/math/add | |
| import { MockConfigManager } from './mock-config-manager'; | |
| import { add } from '@hipponot/soa-math/add' | |
| // … other items … | |
| import { IConfigManager, MockConfigManager } from '@hipponot/soa-config'; |
🤖 Prompt for AI Agents
In .cursor/rules/naming-conventions.mdc at line 15, the import statement uses an
incorrect package scope '@saga-soa'. Update the import to use the correct
'@hipponot/soa-' scope by changing the line to import { add } from
'@hipponot/soa-math/add'. Also, at line 20, update the import from
'@hipponot/config' to '@hipponot/soa-config' to maintain consistent package
namespace usage.
| ARG NPM_TOKEN | ||
| RUN echo "@saga:registry=https://your-registry.com" >> ~/.npmrc | ||
| RUN echo "//your-registry.com/:_authToken=${NPM_TOKEN}" >> ~/.npmrc | ||
|
|
There was a problem hiding this comment.
Registry scope still uses deprecated “@saga”
All .npmrc snippets should switch to @hipponot to match the package rename; otherwise installs will fail.
- RUN echo "@saga:registry=https://your-registry.com" >> ~/.npmrc
+ RUN echo "@hipponot:registry=https://your-registry.com" >> ~/.npmrcAlso applies to: 70-74, 235-237
🤖 Prompt for AI Agents
In docs/docker-registry-examples.md at lines 20 to 23, the npm registry scope
uses the deprecated "@saga". Update all occurrences of "@saga" to "@hipponot" to
reflect the package rename and ensure installs work correctly. Also apply this
change to lines 70-74 and 235-237.
| echo "Building Web App..." | ||
| docker build \ | ||
| --network=host \ | ||
| -t saga-web:latest \ | ||
| -f apps/web/Dockerfile . | ||
|
|
||
| echo "Building Docs..." | ||
| docker build \ | ||
| --network=host \ | ||
| -t saga-docs:latest \ | ||
| -f apps/docs/Dockerfile . | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Build script references removed apps
apps/web and apps/docs were deleted in this PR; the build script will exit with non-zero.
Drop those targets or gate them behind an existence check.
🤖 Prompt for AI Agents
In docs/docker-registry-examples.md around lines 158 to 169, the build script
references the deleted directories apps/web and apps/docs, causing build
failures. Update the script to either remove these build targets entirely or add
checks to verify the existence of these directories before attempting to build,
ensuring the script does not fail when the directories are missing.
| turbo run build --filter=packages/logger | ||
| turbo run build --filter=apps/examples/rest_api | ||
| turbo run build --filter=saga-soa-examples/examples/rest-api | ||
| ``` | ||
| - **Test:** | ||
| ```sh | ||
| turbo run test --filter=packages/config | ||
| turbo run test --filter=apps/examples/rest_api | ||
| turbo run test --filter=saga-soa-examples/examples/rest-api | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Stale paths to example projects
Examples still use the legacy saga-soa-examples/... directories, whereas other docs (and Dockerfiles) now point to apps/examples/.... Running these commands will fail in a clean clone.
- turbo run build --filter=saga-soa-examples/examples/rest-api
+ turbo run build --filter=apps/examples/rest-api
...
- pnpm --filter ./saga-soa-examples/web dev
+ pnpm --filter ./apps/web devAlso applies to: 118-121
🤖 Prompt for AI Agents
In docs/GETTING-STARTED.md around lines 95 to 102 and also lines 118 to 121,
update the example project paths from the legacy saga-soa-examples/...
directories to the current apps/examples/... directories. Replace all
occurrences of saga-soa-examples with apps/examples in the turbo run commands to
ensure they work correctly in a clean clone.
| The exports field in package.json allows more granular support of exports. For example to import only the math function from the package @saga-soa/math you can use the below if it is listed in the package.json exports. | ||
|
|
||
| ``` | ||
| import { add } from @saga/math/add | ||
| import { add } from @saga-soa/math/add | ||
| ``` |
There was a problem hiding this comment.
Namespace inconsistency – still using @saga-soa.
The docs example should now reference @hipponot/math (or @hipponot/soa-math) to stay aligned with the rest of the rename.
-import { add } from @saga-soa/math/add
+import { add } from @hipponot/soa-math/add📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The exports field in package.json allows more granular support of exports. For example to import only the math function from the package @saga-soa/math you can use the below if it is listed in the package.json exports. | |
| ``` | |
| import { add } from @saga/math/add | |
| import { add } from @saga-soa/math/add | |
| ``` | |
| The exports field in package.json allows more granular support of exports. For example to import only the math function from the package @saga-soa/math you can use the below if it is listed in the package.json exports. | |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In human-notes/turborepo-notes.md lines 1 to 5, the example import statement
uses the outdated namespace `@saga-soa`. Update the import path to use the new
namespace `@hipponot/math` (or `@hipponot/soa-math`) to maintain consistency
with the renamed package throughout the documentation.
| "dependencies": { | ||
| "reflect-metadata": "^0.2.2", | ||
| "routing-controllers": "^0.11.2" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Transitive runtime deps don’t belong in a pure “config” package
reflect-metadata and routing-controllers introduce ~4 MB install size and force downstream users to get unnecessary code at runtime.
Move them to devDependencies (or drop altogether) unless they are really required by the exported JSON configs.
- "dependencies": {
- "reflect-metadata": "^0.2.2",
- "routing-controllers": "^0.11.2"
- }
+ "devDependencies": {
+ "reflect-metadata": "^0.2.2",
+ "routing-controllers": "^0.11.2"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dependencies": { | |
| "reflect-metadata": "^0.2.2", | |
| "routing-controllers": "^0.11.2" | |
| } | |
| "devDependencies": { | |
| "reflect-metadata": "^0.2.2", | |
| "routing-controllers": "^0.11.2" | |
| } |
🤖 Prompt for AI Agents
In packages/typescript-config/package.json around lines 21 to 24, the
dependencies "reflect-metadata" and "routing-controllers" are runtime
dependencies that increase install size and add unnecessary code for downstream
users of this config package. Move these two packages from "dependencies" to
"devDependencies" unless they are explicitly required at runtime by the exported
JSON configs. This will keep the package lightweight and avoid forcing runtime
installs of these libraries.
| @@ -1,4 +1,4 @@ | |||
| import { config } from "@saga/eslint-config/react-internal"; | |||
| import { config } from "@saga-soa/eslint-config/react-internal"; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the target namespace for ESLint configuration.
The import was updated from @saga/eslint-config to @saga-soa/eslint-config, but other files in this PR are migrating to @hipponot namespace. Please confirm if this should be @hipponot/soa-eslint-config instead to maintain consistency.
🏁 Script executed:
#!/bin/bash
# Description: Check for ESLint config package references and verify correct namespace
# Expected: Find other ESLint config references to determine consistent naming
echo "Searching for eslint-config package references:"
rg -A 2 -B 2 "eslint-config"
echo -e "\nSearching for @hipponot/soa-eslint-config references:"
rg -A 2 -B 2 "@hipponot.*eslint-config"
echo -e "\nSearching for @saga-soa/eslint-config references:"
rg -A 2 -B 2 "@saga-soa.*eslint-config"Length of output: 6770
Update ESLint config import to @hipponot namespace.
The UI package’s devDependencies and the published config package are both under @hipponot/soa-eslint-config, so the import in packages/ui/eslint.config.mjs must be updated accordingly:
• File: packages/ui/eslint.config.mjs
Change the import source to match the @hipponot/soa-eslint-config package.
-import { config } from "@saga-soa/eslint-config/react-internal";
+import { config } from "@hipponot/soa-eslint-config/react-internal";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { config } from "@saga-soa/eslint-config/react-internal"; | |
| -import { config } from "@saga-soa/eslint-config/react-internal"; | |
| +import { config } from "@hipponot/soa-eslint-config/react-internal"; |
🤖 Prompt for AI Agents
In packages/ui/eslint.config.mjs at line 1, update the import statement to
change the source from "@saga-soa/eslint-config/react-internal" to
"@hipponot/soa-eslint-config/react-internal" to align with the correct package
namespace used in devDependencies and the published config package.
| "name": "@hipponot/soa-ui", | ||
| "version": "0.0.1", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add publishConfig (and ideally repository) to keep publishing consistent
Other packages now declare
"publishConfig": {
"registry": "https://npm.pkg.github.com",
"access": "public"
}Without it, pnpm publish will default to the public npm registry, breaking the monorepo’s single-registry story.
Add the block (and a repository field) so this package publishes to the same place as the others.
"version": "0.0.1",
+ "publishConfig": {
+ "registry": "https://npm.pkg.github.com",
+ "access": "public"
+ },
+ "repository": {
+ "type": "git",
+ "url": "https://github.com/hipponot/saga-soa.git",
+ "directory": "packages/ui"
+ },🤖 Prompt for AI Agents
In packages/ui/package.json around lines 2 to 3, add a "publishConfig" field
with "registry" set to "https://npm.pkg.github.com" and "access" set to "public"
to ensure publishing uses the correct registry. Also add a "repository" field
with the appropriate repository URL to keep metadata consistent with other
packages in the monorepo.
| - 'apps/docs' | ||
| - 'apps/web' |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that apps/docs and apps/web actually exist and still belong in the workspace
If these directories were deleted (the AI-summary hints at their removal), keeping them listed will break pnpm install -r / pnpm -F ... commands with a package manifest not found error.
Remove the entries or restore the folders so the workspace stays consistent.
🏁 Script executed:
# quick sanity-check
fd package.json apps/docs apps/webLength of output: 188
🏁 Script executed:
#!/bin/bash
# Verify existence of apps folder and its subdirectories
if [ -d apps ]; then
echo "apps/ directory found. Contents:"
ls -1 apps
else
echo "apps/ directory not found"
fiLength of output: 78
Remove non-existent workspace entries in pnpm-workspace.yaml
The directories apps/docs and apps/web aren’t present in the repo, so leaving them in pnpm-workspace.yaml will cause package manifest errors when running pnpm install -r or pnpm -F.
Please update pnpm-workspace.yaml to either restore these folders or remove their entries:
• File: pnpm-workspace.yaml (lines 2–3)
• Change:
diff - - 'apps/docs' - - 'apps/web'
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - 'apps/docs' | |
| - 'apps/web' |
🤖 Prompt for AI Agents
In pnpm-workspace.yaml at lines 2 to 3, the entries 'apps/docs' and 'apps/web'
refer to directories that do not exist in the repository. To fix this, remove
these two entries from the workspace configuration to prevent package manifest
errors during pnpm install or pnpm commands.
| "@hipponot/config": ["packages/config/src"], | ||
| "@hipponot/config/*": ["packages/config/src/*"], | ||
| "@hipponot/soa-logger": ["packages/logger/src"], | ||
| "@hipponot/soa-logger/*": ["packages/logger/src/*"] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Path mappings updated correctly for namespace change.
The TypeScript path aliases have been properly updated to reflect the new @hipponot namespace. However, there's an inconsistency in the naming pattern: @hipponot/config vs @hipponot/soa-logger. Please verify this is intentional.
🏁 Script executed:
#!/bin/bash
# Check if the naming inconsistency is intentional by examining package.json files
echo "Checking package names in the monorepo:"
fd package.json packages/ --exec cat {} | grep -E '"name":\s*"@hipponot/' | sortLength of output: 423
Fix TS path alias for config package
The tsconfig.json path mappings use @hipponot/config, but the packages/config/package.json defines the name as @hipponot/soa-config. Please update the aliases to match the actual package name (@hipponot/soa-config). The logger mappings are already correct.
• File: tsconfig.json
Lines: ~6–9
- "@hipponot/config": ["packages/config/src"],
- "@hipponot/config/*": ["packages/config/src/*"],
+ "@hipponot/soa-config": ["packages/config/src"],
+ "@hipponot/soa-config/*": ["packages/config/src/*"],
"@hipponot/soa-logger": ["packages/logger/src"],
"@hipponot/soa-logger/*": ["packages/logger/src/*"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "@hipponot/config": ["packages/config/src"], | |
| "@hipponot/config/*": ["packages/config/src/*"], | |
| "@hipponot/soa-logger": ["packages/logger/src"], | |
| "@hipponot/soa-logger/*": ["packages/logger/src/*"] | |
| "@hipponot/soa-config": ["packages/config/src"], | |
| "@hipponot/soa-config/*": ["packages/config/src/*"], | |
| "@hipponot/soa-logger": ["packages/logger/src"], | |
| "@hipponot/soa-logger/*": ["packages/logger/src/*"] |
🤖 Prompt for AI Agents
In tsconfig.json around lines 6 to 9, the path aliases for the config package
use "@hipponot/config" but the actual package name is "@hipponot/soa-config".
Update the aliases to "@hipponot/soa-config" and "@hipponot/soa-config/*" to
match the package.json name, leaving the logger aliases unchanged.
1 participant
Summary by CodeRabbit
New Features
Refactor
@saga-soato@hipponotfor consistency.Bug Fixes
Chores
Documentation