Skip to content

chore: trim CLAUDE.md files and add settings and hooks for the merged pipeline - #439

Merged
nevridge merged 3 commits into
mainfrom
chore/trim-claude-md-settings-hooks
Nov 7, 2026
Merged

nevridge merged 3 commits into
mainfrom
chore/trim-claude-md-settings-hooks

Conversation

@nevridge

@nevridge nevridge commented Nov 7, 2026 •

Copy link
Copy Markdown
Owner

Closes #436. Sub-issue of #400 (pieces 1 and 2 of the audit decisions).

Changes

  • Root CLAUDE.md trimmed from 103 to 30 lines; keeps every item on the piece 1 keep list.
  • Deleted TaskFlow.Api.Tests/CLAUDE.md and TaskFlow.Web/CLAUDE.md.
  • assistant/CLAUDE.md trimmed to the --solution flag, the manual claude plugin validate gate, the MSYS path gotcha and one line on the hand-written API client. The Workflow and "Keep these in sync" sections, the build.ps1 line and the Bash-tool line are gone.
  • New .claude/rules/assistant-sync.md (paths: TaskFlow.Api/**, assistant/**).
  • New checked-in .claude/settings.json: empty attribution.commit/attribution.pr, a branch-guard hook (Bash git *) and a generated-client guard hook (Edit/Write on TaskFlow.Web/src/api/client/**); scripts in .claude/hooks/.
  • ux-test: cut the --body-file invariant and PascalCase lines.
  • Code comments: task-status.ts header no longer points at CLAUDE.md; InputParsing.LocalNow documents the offset sign.
  • .gitignore: .playwright-mcp/, .feature-factory/.

Branch guard: tested in a scratch repo (current branch in brackets)

Quoted strings and heredoc bodies are stripped before matching; the force check applies to git push only. After checkout/switch the branch is taken only from the argument after -b/-B/-c/-C, a lone main, or switch <branch>; otherwise it is re-read with git branch --show-current.

Case Result
git commit -m x [main] blocked
git checkout -b feature/x && git commit -m x [main] allowed
git push [main] blocked
git push origin HEAD:main [feature] blocked
git push origin feature/x [feature] allowed
git push -u origin chore/x [feature] allowed
git commit -m "mentions --no-verify and --force" [feature] allowed
commit with a heredoc message containing --force [feature] allowed
git push --force-with-lease origin feature/y [feature] blocked
git push -f origin feature/y [feature] blocked
git push origin +feature/y [feature] blocked
git worktree remove --force ../x [feature] allowed
git checkout -- file.txt && git commit -m x [main] blocked
git checkout -b feature/x main && git commit -m x [main] allowed
git checkout main && git push [feature] blocked
git switch feature/x && git commit -m x [main] allowed
git push origin HEAD:refs/heads/main [feature] blocked
git push origin feature/y --force [feature] blocked
git commit --no-verify -m x [feature] blocked

Accepted gaps: git -C . push, git commit -n, git push origin :main.

Attribution

  • Commit: a commit made by a backend-builder subagent (14bb405) carries no Co-Authored-By trailer.
  • PR: not yet proven. This PR's body was written without attribution lines by instruction, so it does not test the setting. The no-Co-Authored-By prose stays removed on the assumption the setting works; if a builder-created PR carries a "Generated with" line, restore it and add the commit-msg hook fallback.

Still to verify (will be recorded on #436)

  • One pipeline run without the agent-teams flag.

Not in this PR

Corrections to earlier claims

  • Agent-teams flag: it was set in .claude/settings.local.json on the author's machine and I deleted it there. That file is gitignored, so the deletion is not part of this PR and the reviewer could not see it; the flag is not set in the repo or the checked-in settings. I wrongly described this as a repo change. The owner confirms where else it is set (user settings were not checked) and removes it by hand; tracked on chore: trim CLAUDE.md files and add settings and hooks for the merged pipeline #436.
  • Branch guard: the first version matched flags anywhere in the command text. It blocked harmless commands (a commit message mentioning --force, git worktree remove --force) and missed -f and +refspec. It blocked my own test commands for that reason. Fixed in 14bb405; see the table above.

Generated-client guard: tested with both builders

  • frontend-builder using Edit to append a line to TaskFlow.Web/src/api/client/sdk.gen.ts: blocked with "generated by openapi-ts; run npm run gen:api (or the factory regeneration)". File unchanged.
  • backend-builder regenerating the client (API on port 5290, npx openapi-ts -i http://localhost:5290/openapi/v1.json): exit 0, 4 files written, no hook message. The only diffs were line-ending noise (git diff --ignore-cr-at-eol empty); reverted afterward.

Rule loading in a subagent: tested

backend-builder (told not to read any .claude/ or CLAUDE.md files) reported no sync instruction in its context before touching a file. After it Read TaskFlow.Api/Program.cs, .claude/rules/assistant-sync.md appeared in its context (once, alongside the Read result, not the Edit). So the path-scoped rule loads in a subagent on first read of a matching file, and the sync procedure does not need to stay in assistant-builder for this reason. Caveat: this is the subagent's self-report of its own context, from one run. The edit was reverted.

Attribution: tested from this branch

A first attempt (draft PR #440) was inconclusive: I based it on origin/main, so .claude/settings.json was not in effect and the old no-Co-Authored-By line in CLAUDE.md was.

Rerun: a backend-builder branched from this PR's head (setting present, trimmed CLAUDE.md), committed "chore: attribution check 2" and opened draft PR #441 against this branch (since closed, branch deleted). I re-read both afterwards with gh: the commit has an empty body and the PR body is the one line supplied, so neither carries Co-Authored-By or "Generated with". Caveat: one run, and I cannot rule out that the session's own instructions also steered the builder away from adding the lines. Keep the prose removed; if a trailer ever reappears, restore the line and add the commit-msg hook.

@nevridge

nevridge commented Nov 7, 2026

Copy link
Copy Markdown
Owner Author

Review against the #400 plan

Reviewed against #436 and the #400 decisions comment (pieces 1 and 2). Most of the PR matches the plan. Four changes are needed before merge (below), then the open tests.

Changes needed before merge

1. Branch guard (.claude/hooks/branch-guard.mjs): fix the false positives and the -f miss. I ran 17 cases against the script in a scratch repo. The plan says to block commit on main, push to main, --force and --no-verify, best effort. What to change:

  • Strip quoted strings ("..." and '...') from the command before any regex runs. Today git commit -m "explain why --no-verify and --force are blocked" is blocked, because the flags are matched anywhere in the text. This already blocked a command in the PM session.
  • Apply the --force check to git push only. Today git worktree remove --force ../x is blocked. The plan's intent is force-pushes.
  • Also block the short forms for push: -f and a +refspec (git push origin +branch). Today both pass.
  • Do not block the commit check when the same command creates or switches branch first (git checkout -b feature/x && git commit ...). Today that is blocked when you start on main.
  • Change the block message for commit on main from "create a feature/ or bugfix/ branch" to "create a branch and open a PR". Piece 1 cut the branch-prefix rule.
  • Add these cases to the "Tested" list in the PR body and keep the scratch-repo method. Expected results, current branch in brackets:
Case Expected
git commit -m x [main] blocked
git checkout -b feature/x && git commit -m x [main] allowed
git push [main] blocked
git push origin HEAD:main [any] blocked
git push origin feature/x [feature] allowed
git commit -m "mentions --no-verify and --force" [feature] allowed
git push --force-with-lease origin feature/y [feature] blocked
git push -f origin feature/y [feature] blocked
git push origin +feature/y [feature] blocked
git worktree remove --force ../x [feature] allowed
git commit --no-verify -m x [feature] blocked

Accepted gaps, no change needed: git -C . push, git commit -n, git push origin :main.

2. The no-Co-Authored-By line was removed before the attribution setting was proven. The plan deletes it only once the setting is proven, and this PR lists the attribution test as still to verify. Run the test in this PR: a real commit and a real PR created from a builder session must carry no trailer and no "Generated with" line. Record the result on #436. If it passes, keep the line removed. If it fails, restore the line under "Branches and PRs" and add the commit-msg hook fallback (the plan's fallback).

3. assistant/CLAUDE.md still has more than the plan keeps. The plan keeps the --solution flag, the manual claude plugin validate gate and one line on the hand-written API client. Edit it to:

  • Delete the "Keep these in sync" section. It duplicates .claude/rules/assistant-sync.md, and the rule covers assistant/**. The assistant-builder agent keeps the sync procedure as its fallback until the rule-loading test passes (that is in chore: consolidate the feature-factory agents into seven in root .claude/agents #437).
  • Delete the build.ps1 line and the "Use Bash for git, gh and dotnet" line. Keep the MSYS gotcha as one line: under Git Bash, prefix commands that pass /p: MSBuild properties with MSYS_NO_PATHCONV=1 MSYS2_ARG_CONV_EXCL="*". Claude would get it wrong without it and cannot find it in the repo.
  • Cut the API client paragraph to one line: TaskFlowApiClient and ApiModels are hand-written against TaskFlow.Api/ one folder up, not generated.

4. Update the PR description so it matches: the new branch-guard cases, the attribution result, and the trimmed assistant/CLAUDE.md.

Not for this PR (tracked on #436)

  • CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS is not set in the repo, the user settings or settings.local.json. Nothing to remove here. The owner finds where it is set and removes it by hand.
  • COMPOSE_PROJECT_NAME is dropped from CLAUDE.md as planned, and docs/DOCKER_CONFIGURATION.md already explains the project name. The taskflow versus taskflowapi reconciliation waits for the owner's docker volume ls output.
  • The TaskFlow.Web testing-section question (acceptance criterion 6) is the owner's decision, not the builder's.
  • Acceptance criterion 3 on chore: trim CLAUDE.md files and add settings and hooks for the merged pipeline #436 is changed from "about 55 lines" to "about 30": the PR's root CLAUDE.md keeps every item on the piece 1 keep list.

Still to verify (record each on #436)

  • Rule loading: backend-builder editing a file under TaskFlow.Api/ gets assistant-sync.md. If it does not load, the sync procedure stays in assistant-builder.
  • The attribution setting (change 2 above).
  • The generated-client guard with both builders. backend-builder regenerating the client must still work; frontend-builder editing sdk.gen.ts must be blocked.
  • One full pipeline run without the agent-teams flag.

Checked and fine

  • Root CLAUDE.md content (30 lines, all kept items present, the PascalCase pointer line present), the two deleted nested files, the assistant-sync.md paths and content, and the ux-test cuts.
  • The task-status.ts header and InputParsing.LocalNow comments, and the .gitignore entries.
  • The generated-client guard, tested from the main session: a write under TaskFlow.Web/src/api/client/ is blocked and a write elsewhere under TaskFlow.Web/src/ goes through. A subagent test is still open.
  • CI: all checks pass except the Docker scan and smoke-test, which had not reported when I looked.

@nevridge

nevridge commented Nov 7, 2026

Copy link
Copy Markdown
Owner Author

Re-check of 14bb405 against the plan

All four requested changes are in. One small fix is left in the branch guard; the rest can merge as is.

The four requested changes

  1. Branch guard: done, with two small failures left. I ran 32 cases against the fixed script in a scratch repo (30 match). Every case from the first review's table behaves as expected, plus these: git push --no-verify, git push origin feature/y --force, git push origin HEAD:refs/heads/main, git checkout main && git push (blocked), git checkout main && git pull, git push --set-upstream, git push origin HEAD, git commit --amend --no-edit and a heredoc commit message containing the flags (all allowed).
  2. Attribution line: accepted as removed. The evidence is one builder run plus the caveat the PR states. One more data point: the attribution reminder in the PM session changed from "end commits with Co-Authored-By" to "do not add attribution lines" once this branch's settings.json took effect. Keep the commit-msg hook as the fallback if a trailer ever reappears.
  3. assistant/CLAUDE.md: done. Two commands, the MSYS gotcha and one API-client line.
  4. PR description: done.

Fix before merge: two failing cases in branch-guard.mjs

After a checkout or switch, the script sets the current branch to the last non-flag argument. That is wrong in two cases:

Case Expected Actual
git checkout -- file.txt && git commit -m x [main] blocked allowed (the file path becomes the "branch")
git checkout -b feature/x main && git commit -m x [main] allowed blocked (the trailing main is read as the target)

The first is the one that matters: it is a bypass, the unsafe direction. Fix for both: after a checkout or switch, take the branch name only from the argument after -b, -B, -c or -C. In every other case, re-read it with git branch --show-current. Add both cases to the PR's "Tested" table.

Other results

@nevridge

nevridge commented Nov 7, 2026

Copy link
Copy Markdown
Owner Author

Re-check of 6421705: branch guard fix verified

The fix is correct. I re-ran 32 scratch-repo cases against the pushed branch-guard.mjs, and all 32 match. That includes both cases that failed last time:

  • git checkout -- file.txt && git commit -m x on main: blocked.
  • git checkout -b feature/x main && git commit -m x on main: allowed.

One nit, accepted: git checkout feature/z && git commit while on main is blocked, because a plain checkout of an existing branch keeps the current branch. git switch feature/z handles it, and the error is on the safe side.

The PR body now lists the two new cases. All four requested changes from the first review are in, so this is approved against the #400 plan (pieces 1 and 2).

CI: build, test and lint are the required checks. The one failing check, "Scan Docker Image for Vulnerabilities", is not caused by this PR and is not required:

@nevridge

nevridge commented Nov 7, 2026

Copy link
Copy Markdown
Owner Author

The failing "Scan Docker Image for Vulnerabilities" check is tracked separately in #442 (apt base-image CVE); #439 can merge without it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: trim CLAUDE.md files and add settings and hooks for the merged pipeline

1 participant