feat(session-3-block-3): GET /api/product endpoint + product.yaml real#3
Conversation
…FromFile ProductConfigError gains an optional code?: string constructor parameter. parseProductConfigFromFile passes fsErr.code when wrapping the fs exception, so callers can do err.code === 'ENOENT' without reaching into err.cause. This is more robust than message-string matching — the code property is stable regardless of message wording changes in the parser. Test updated: now asserts err.code === 'ENOENT' in addition to instanceof and message checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…owledge repo
Splits app.ts (HTTP routes, testable in Node.js) from index.ts (Bun server
wiring + WS). Tests import app.ts directly without pulling in hono/bun.
GET /api/product:
- 500 if HELM_KNOWLEDGE_REPO_PATH is not set
- reads {HELM_KNOWLEDGE_REPO_PATH}/.helm/product.yaml via parseProductConfigFromFile
- 404 if file missing (detected via err.code === 'ENOENT', not string matching)
- 500 with field path if schema is invalid (ProductConfigError message)
- 200 + Product JSON on success
4 tests using app.request() (no server spin-up):
valid config → 200, missing file → 404, invalid schema → 500+field path,
missing env var → 500. beforeEach/afterEach snapshot+restore process.env.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents the env var required by GET /api/product. Lives in apps/api/ (not root) because Bun reads .env from the process CWD, which is apps/api/ when Turbo runs the dev server. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Documents where .env files live (apps/api/.env, not root) and why — Bun reads .env from the process CWD which is apps/api/ when Turbo runs. Prevents future agents/humans from looking for .env in the wrong place. Lists HELM_KNOWLEDGE_REPO_PATH as the first required server variable. Updates helm-knowledge reference (no longer 'próximamente'). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds a ChangesProduct Endpoint and Local Development
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/routes/product.ts`:
- Around line 21-23: The error handling currently returns the absolute
configPath to clients (in the ENOENT branch), leaking host paths; instead, keep
the absolute path in server logs (e.g., console.error or your logger) and return
a stable relative location or friendly message to the client. Update the ENOENT
branch that references configPath so it logs the full configPath server-side
(using console.error or processLogger) and change the c.json response to a fixed
relative identifier such as "product.yaml in config directory" or
"config/product.yaml" without including configPath; keep references to
configPath only in logs and not in the HTTP response.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c547625e-52a5-421e-8810-ed6f47e4c896
📒 Files selected for processing (9)
AGENT.mdapps/api/.env.exampleapps/api/package.jsonapps/api/src/app.tsapps/api/src/index.tsapps/api/src/routes/product.test.tsapps/api/src/routes/product.tspackages/shared/src/config/product-parser.test.tspackages/shared/src/config/product-parser.ts
The ENOENT branch was returning configPath (absolute host filesystem path)
to the HTTP client. Server-side debug info is kept via console.error; the
client now receives a stable relative identifier instead:
'$HELM_KNOWLEDGE_REPO_PATH/.helm/product.yaml'
Test assertions already matched on substrings ('not found', '.helm/product.yaml')
so no test changes required.
Addresses CodeRabbit review comment on PR #3.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…response
POST /api/items was forwarding err.message directly to the client on
product config load failure, which could expose filesystem paths from
ProductConfigError ('Cannot read file: /path/...').
Now logs the full error server-side with console.error and returns the
generic 'Failed to load product config' message — consistent with the
path-leak fix applied to /api/product in PR #3.
Test updated: no longer checks for 'HELM_KNOWLEDGE_REPO_PATH' in the
response body; checks for the generic message instead.
Addresses CodeRabbit review comment on PR #6.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Wires
@helm/shared'sparseProductConfigFromFileinto the API, adds E2E config loading from the knowledge repo, and creates the first real.helm/product.yamlinlhpaul/helm-knowledge(committed separately to that repo's main).4 commits:
refactor(@helm/shared): preserve fs error codes in parseProductConfigFromFile—ProductConfigErrorgainscode?: string;parseProductConfigFromFilepassesfsErr.codeso callers checkerr.code === 'ENOENT'directly without inspectingcause. More robust than string-matching the message.feat(@helm/api): add GET /api/product endpoint loading config from knowledge repo— Splitsapp.ts(HTTP routes, testable in Node.js/Vitest) fromindex.ts(Bun server + WS wiring).GET /api/productreadsHELM_KNOWLEDGE_REPO_PATHenv var, loads{path}/.helm/product.yamlviaparseProductConfigFromFile, returns 200/404/500 with typed JSON errors. 4 tests usingapp.request().chore(@helm/api): add .env.example for HELM_KNOWLEDGE_REPO_PATH— Documents that.envlives inapps/api/(not root) because Bun reads from process CWD.docs: add 'Configuración local' section to AGENT.md— Documents per-package.envconvention, listsHELM_KNOWLEDGE_REPO_PATHas first required var.Test plan
pnpm turbo run test→ 30/30 (4 api + 14 storage + 12 shared)pnpm turbo run build→ 4/4, tsc --noEmit clean, api bundles 134 modulespnpm turbo run lint→ 4/4, 0 errorsHELM_KNOWLEDGE_REPO_PATH=/Users/lhpaul/Git/Helm/helm-knowledge pnpm dev+curl :3001/api/product→ 200 with Helm Product JSON (verified after PR)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Bug Fixes / Improvements