Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .claude/hooks/block-npm-publish.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/usr/bin/env bash
# PreToolUse hook (matcher: Bash). Denies any command that invokes `npm publish`.
# Heredoc bodies are stripped before matching so prose (e.g. a commit message
# that mentions "npm publish") inside `git commit -m "$(cat <<'EOF' ... EOF)"`
# isn't mistaken for an actual invocation.
set -euo pipefail

cmd=$(jq -r '.tool_input.command')

stripped=$(printf '%s\n' "$cmd" | awk '
BEGIN { indelim = 0 }
{
if (indelim) {
line = $0
sub(/^[[:space:]]+/, "", line)
if (line == delim) { indelim = 0 }
next
}
if (match($0, /<<-?[[:space:]]*['"'"'"]?[A-Za-z_][A-Za-z0-9_]*['"'"'"]?/)) {
tok = substr($0, RSTART, RLENGTH)
gsub(/<<-?[[:space:]]*/, "", tok)
gsub(/['"'"'"]/, "", tok)
delim = tok
indelim = 1
}
print
}
')

if printf '%s' "$stripped" | grep -qiE '(^|[;&|]|[[:space:]])npm(\.cmd)?[[:space:]]+publish([[:space:]]|$)'; then
printf '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"npm publish is blocked by project policy (.claude/hooks/block-npm-publish.sh)."}}'
fi

exit 0
26 changes: 26 additions & 0 deletions .claude/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
{
"hooks": {
"PreToolUse": [
{
"matcher": "Bash",
"hooks": [
{
"type": "command",
"command": "bash .claude/hooks/block-npm-publish.sh"
}
]
}
],
"PostToolUse": [
{
"matcher": "Write|Edit",
"hooks": [
{
"type": "command",
"command": "jq -r '.tool_response.filePath // .tool_input.file_path' | { read -r f; npx prettier --write \"$f\"; } 2>/dev/null || true"
}
]
}
]
}
}
4 changes: 4 additions & 0 deletions .prettierrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"tabWidth": 2,
"useTabs": false
}
47 changes: 47 additions & 0 deletions NOTES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# NOTES

## The plan

The approved plan added a `PUT /users/:id` endpoint by extending the two existing files: an
`updateUser(id, { name, email })` helper in `db/store.js` that mirrors `createUser` and returns
`undefined` for an unknown id, and a route handler in `routes/users.js` that validates the body with
the same truthiness check `POST /users` already uses, then 404s on an unknown id. I confirmed the
validation strategy (match the existing simple check rather than add stricter type/format checks) and
the scope (full branch → commits → NOTES → review → PR, not just the code) before approving; I didn't
edit anything else in the plan itself.

## Model

Opus, for the planning and implementation. The task is small, but getting the contract right from the
three grading tests (200/404/400) and matching the codebase's existing conventions exactly benefited
from a careful read of the existing route/store code before writing anything.

## Commit split

Three commits: the store helper, then the route that depends on it, then this file. Store-then-route
keeps each commit buildable and reviewable on its own — the second commit is the one that actually
turns `tests/update-user.test.js` green, so `npm test` was run right before making it. NOTES.md is a
separate, non-code commit.

## What the review caught

A self-review (via the `code-review` skill) flagged that the `if (!name || !email)` required-field
check in the new `PUT /users/:id` handler is now duplicated verbatim from `POST /users`, and suggested
extracting a shared validator. I decided to leave it as-is: it's a single-line check repeated in two
call sites in a file with no existing validation layer, and the codebase has no controller/validator
abstraction anywhere else — adding one here for a one-liner would be more indirection than the
duplication costs. Everything else the review checked (the 404 path, id coercion via `Number(...)`,
that the store owns the mutation rather than the route touching the array) was already fine.

## My additional Notes
In a short NOTES.md, answer in a few sentences each:

What was in the plan you approved, and did you edit anything before approving?
- I selected step by step approval so I could better check what will done
Which model did you choose, and why?
- I choose Opus model because of quite complicated task in spite of costs
How did you split your commits, and why that way?
- commits was plitted to clear buildable and testable parts
What did your review catch — or confirm was already fine?
- the base code and logic was fine and modifications and testings clear

14 changes: 13 additions & 1 deletion db/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,16 @@ function createUser({ name, email }) {
return user;
}

module.exports = { getAllUsers, getUserById, createUser };
function updateUser(id, { name, email }) {
const user = getUserById(id);

if (!user) {
return undefined;
}

user.name = name;
user.email = email;
return user;
}

module.exports = { getAllUsers, getUserById, createUser, updateUser };
17 changes: 17 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,16 @@
"dev": "node --watch server.js",
"start": "node server.js",
"test": "node --test",
"lint": "eslint ."
"lint": "eslint .",
"format": "prettier --write ."
},
"license": "MIT",
"dependencies": {
"express": "^4.19.2"
},
"devDependencies": {
"eslint": "^8.57.0",
"prettier": "^3.9.6",
"supertest": "^7.0.0"
}
}
18 changes: 18 additions & 0 deletions routes/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,22 @@ router.post("/", (req, res) => {
res.status(201).json(user);
});

// PUT /users/:id — update an existing user; name and email are required
router.put("/:id", (req, res) => {
const { name, email } = req.body;

if (!name || !email) {
return res.status(400).json({ error: "name and email are required" });
}

const id = Number(req.params.id);
const user = store.updateUser(id, { name, email });

if (!user) {
return res.status(404).json({ error: "User not found" });
}

res.json(user);
});

module.exports = router;