diff --git a/.claude/commands/review-route.md b/.claude/commands/review-route.md new file mode 100644 index 0000000..2cdf654 --- /dev/null +++ b/.claude/commands/review-route.md @@ -0,0 +1,24 @@ +Review the route file for `$ARGUMENTS` against this project's conventions. If $ARGUMENTS looks like a file path, read it directly. If it looks like a resource name (e.g. "products"), look for `routes/$ARGUMENTS.js`. + +Check each of the following and report pass / fail / not-applicable for each item: + +**Error responses** +- Every error response uses the shape `{ "error": "message" }` (no other keys, no `message:` top-level field) +- 400 is returned when required input is missing or invalid +- 404 is returned when a record is not found +- 201 is returned for successful POST (not 200) + +**Route hygiene** +- Every error branch uses `return res.status(NNN).json(...)` — early return, not fall-through +- IDs from `req.params` are parsed with `Number(req.params.id)` before being passed to the store +- No in-route state — all reads and writes go through `db/store.js` functions + +**Store alignment** +- Read `db/store.js` and confirm each store function called by this route actually exists there +- No raw array access or direct mutation inside the route file + +**Test coverage** +- A test file exists at `tests/$ARGUMENTS.test.js` (or similar) +- If it exists: confirm there is a `test.beforeEach(() => store.reset())` call before any test that writes data + +At the end, give a one-line verdict: **all good**, **minor issues** (list them), or **needs fixes** (list them). diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..5fd0973 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,20 @@ +{ + "permissions": { + "allow": [ + "mcp__fetch__fetch" + ] + }, + "hooks": { + "PostToolUse": [ + { + "matcher": "Write|Edit", + "hooks": [ + { + "type": "command", + "command": "jq -r '.tool_input.file_path // .tool_response.filePath' | { read -r f; case \"$f\" in *.js) ./node_modules/.bin/eslint --fix \"$f\";; esac; } 2>/dev/null || true" + } + ] + } + ] + } +} diff --git a/.claude/skills/add-resource/SKILL.md b/.claude/skills/add-resource/SKILL.md new file mode 100644 index 0000000..33ae28c --- /dev/null +++ b/.claude/skills/add-resource/SKILL.md @@ -0,0 +1,152 @@ +--- +name: add-resource +description: "Add a resource to this Express API — route file, store helpers, tests, and server mount. Use when the request is to add a new endpoint group or resource (e.g. 'add a products route', 'create a /orders resource')." +--- + +Follow these patterns exactly when adding a new resource to this API. + +## 1. Store functions — `db/store.js` + +Add four functions for the new resource. IDs are auto-incremented integers from a module-level `nextId` variable. `reset()` must re-seed the new resource alongside users. + +```js +let items = []; +let nextItemId = 1; + +function listItems() { + return items; +} + +function getItem(id) { + return items.find((item) => item.id === id); +} + +function createItem({ /* required fields */ }) { + const item = { id: nextItemId, /* fields */ }; + nextItemId += 1; + items.push(item); + return item; +} + +function updateItem(id, fields) { + const item = getItem(id); + if (!item) return undefined; + if (fields.fieldA !== undefined) item.fieldA = fields.fieldA; + return item; +} +``` + +Export all four alongside the existing exports. Update `seed()` and `reset()` to include the new resource. + +## 2. Route file — `routes/.js` + +```js +const express = require('express'); +const store = require('../db/store'); + +const router = express.Router(); + +// GET / — list all. +router.get('/', (req, res) => { + res.json(store.listItems()); +}); + +// GET //:id — fetch one, or 404. +router.get('/:id', (req, res) => { + const item = store.getItem(Number(req.params.id)); + if (!item) { + return res.status(404).json({ error: 'Item not found' }); + } + return res.json(item); +}); + +// POST / — create. Requires . +router.post('/', (req, res) => { + const { fieldA, fieldB } = req.body; + if (!fieldA || !fieldB) { + return res.status(400).json({ error: 'fieldA and fieldB are required' }); + } + const item = store.createItem({ fieldA, fieldB }); + return res.status(201).json(item); +}); + +// PUT //:id — update existing. +router.put('/:id', (req, res) => { + const { fieldA, fieldB } = req.body; + if (fieldA === undefined && fieldB === undefined) { + return res.status(400).json({ error: 'fieldA or fieldB is required' }); + } + const item = store.updateItem(Number(req.params.id), { fieldA, fieldB }); + if (!item) { + return res.status(404).json({ error: 'Item not found' }); + } + return res.json(item); +}); + +module.exports = router; +``` + +Key rules: +- Always `return` early on error responses so the success path doesn't run. +- Error shape is always `{ "error": "message" }` — never any other key. +- `POST` returns 201; all other success responses return 200 (default). +- Parse `:id` with `Number(req.params.id)` before passing to the store. + +## 3. Mount in `server.js` + +```js +const itemsRouter = require('./routes/'); +// ... +app.use('/', itemsRouter); +``` + +Add the require alongside the existing requires, and the `app.use` alongside the existing mounts. + +## 4. Test file — `tests/.test.js` + +```js +const test = require('node:test'); +const assert = require('node:assert'); +const request = require('supertest'); +const app = require('../server'); +const store = require('../db/store'); + +test.beforeEach(() => store.reset()); + +test('GET / returns the seeded list', async () => { + const res = await request(app).get('/'); + assert.equal(res.status, 200); + assert.ok(Array.isArray(res.body)); +}); + +test('GET //:id returns 404 for a missing item', async () => { + const res = await request(app).get('//999'); + assert.equal(res.status, 404); +}); + +test('POST / creates an item', async () => { + const res = await request(app) + .post('/') + .send({ fieldA: 'value', fieldB: 'value' }); + assert.equal(res.status, 201); + assert.equal(res.body.fieldA, 'value'); + assert.ok(res.body.id); +}); + +test('PUT //:id updates an existing item', async () => { + const res = await request(app).put('//1').send({ fieldA: 'new' }); + assert.equal(res.status, 200); + assert.equal(res.body.fieldA, 'new'); +}); + +test('PUT //:id returns 404 for a missing item', async () => { + const res = await request(app).put('//999').send({ fieldA: 'x' }); + assert.equal(res.status, 404); +}); +``` + +Key rules: +- `test.beforeEach(() => store.reset())` is always first — ensures each test starts from seed data. +- Use Node's built-in `node:test` and `node:assert` — no third-party test framework. +- Use `supertest` against the exported `app` — never start the server separately. +- One `assert` per observable outcome; don't assert implementation details. diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 0000000..6591015 --- /dev/null +++ b/.mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "fetch": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-fetch"] + } + } +} diff --git a/NOTES.md b/NOTES.md new file mode 100644 index 0000000..bcae2fb --- /dev/null +++ b/NOTES.md @@ -0,0 +1,21 @@ +# Notes + +## MCP server + +The fetch server (`@modelcontextprotocol/server-fetch`) was connected at project scope via `.mcp.json`. It is useful here because the API's documentation and any external references can be fetched without leaving the coding context — useful for looking up specs or checking example payloads mid-task. The permission rule in `.claude/settings.json` allows only `mcp__fetch__fetch`, scoping access to the read-only fetch tool and blocking any other tools the server might expose. + +## Skill + +The `add-resource` skill captures the repeated work of scaffolding a new REST resource: creating a route file, adding store functions, writing tests, and mounting the router in `server.js`. Without a skill, each new resource requires the same sequence of steps across four files with nothing to enforce consistency. The description was written to name the concrete action ("add a new REST resource") and reference the project's own patterns, so the model resolves it when asked to add a product, order, or any new entity. + +## Custom command + +The `/review-route` command runs a structured checklist against a route file — error shape, status codes, early returns, store alignment, and test coverage — and returns a one-line verdict. It is worth a shortcut because the same checklist applies to every route in the project, the review is mechanical enough to automate, and running it before a commit catches convention drift that is easy to miss in a quick read. + +## Hook + +A `PostToolUse` hook was added to `.claude/settings.json` that runs `eslint --fix` after every `Write` or `Edit` to a `.js` file. It reacts rather than prevents: the edit lands first, then ESLint cleans up any fixable style issues automatically. The event is `PostToolUse` with matcher `Write|Edit`, so it fires after file writes regardless of which route or store file was touched. + +## Headless run + +`claude -p` was used to add the `DELETE /users/:id` route and its `deleteUser` store function. The `--allowedTools` flag was set to `Read,Edit,Bash(npm test)`, which locked the subprocess to reading files, making edits, and running the test suite — nothing else. That constraint meant the task could not install packages, run git, start the server, or take any action outside the narrow scope of implementing and verifying the feature. diff --git a/db/store.js b/db/store.js index 4c92e75..7de3bd1 100644 --- a/db/store.js +++ b/db/store.js @@ -36,9 +36,16 @@ function updateUser(id, fields) { return user; } +function deleteUser(id) { + const user = getUser(id); + if (!user) return undefined; + users.splice(users.indexOf(user), 1); + return user; +} + // Reset to the seed data. Used by the tests so each one starts clean. function reset() { seed(); } -module.exports = { listUsers, getUser, createUser, updateUser, reset }; +module.exports = { listUsers, getUser, createUser, updateUser, deleteUser, reset }; diff --git a/routes/users.js b/routes/users.js index 8945477..0a7a9d6 100644 --- a/routes/users.js +++ b/routes/users.js @@ -10,7 +10,11 @@ router.get('/', (req, res) => { // GET /users/:id — fetch one user, or 404 if it doesn't exist. router.get('/:id', (req, res) => { - const user = store.getUser(Number(req.params.id)); + const id = Number(req.params.id); + if (!Number.isInteger(id)) { + return res.status(400).json({ error: 'id must be an integer' }); + } + const user = store.getUser(id); if (!user) { return res.status(404).json({ error: 'User not found' }); } @@ -29,15 +33,32 @@ router.post('/', (req, res) => { // PUT /users/:id — update an existing user (added in Project 2). router.put('/:id', (req, res) => { + const id = Number(req.params.id); + if (!Number.isInteger(id)) { + return res.status(400).json({ error: 'id must be an integer' }); + } const { name, email } = req.body; if (name === undefined && email === undefined) { return res.status(400).json({ error: 'name or email is required' }); } - const user = store.updateUser(Number(req.params.id), { name, email }); + const user = store.updateUser(id, { name, email }); if (!user) { return res.status(404).json({ error: 'User not found' }); } return res.json(user); }); +// DELETE /users/:id — remove a user, or 404 if not found. +router.delete('/:id', (req, res) => { + const id = Number(req.params.id); + if (!Number.isInteger(id)) { + return res.status(400).json({ error: 'id must be an integer' }); + } + const user = store.deleteUser(id); + if (!user) { + return res.status(404).json({ error: 'User not found' }); + } + return res.status(204).send(); +}); + module.exports = router;