Repository navigation
docs+ci: resolve conflicting agent instructions, make pnpm verify match CI - #269
Merged
Merged
Conversation
…closes #267) Five conflicts between AGENTS.md, CLAUDE.md and the code, each of which could send an agent the wrong way: - Issue closing: "close immediately after committing" contradicted both `closes #N` in the commit message and the draft-PR review flow. Issues now close on merge; hand-closing at commit time is called out as wrong. - AGENTS.md violated its own "no architecture in this file" rule with ~20 lines duplicated from docs/system-architecture.md, including a second copy of the playerFiles glob list. Reduced to the rule-shaped lines, with the architecture referenced by path. - CLAUDE.md violated its own "nothing here unless unique to Claude Code" rule with a general workflow section. Only the `FROM @claude:` prefix and the Write-tool control-character gotcha remain. - Commit rules were split across both files and ambiguous on `main`. Now consolidated in AGENTS.md under "Branching and commits": feature branch is the default, and committing or pushing on `main` each need their own explicit ask. - `pnpm verify` was weaker than CI: it ran only lint:boundary, while CI's unit job also ran test:integration and AGENTS.md separately demanded a clean full lint. A green verify could still fail CI. verify is now typecheck + lint + test + test:integration, and CI's unit job gains the full lint step, so the two genuinely match. lint:boundary stays a separate script for CI's isolated boundary signal. Gate run locally: typecheck, full lint, 166 frontend + hub unit tests, and integration tests all green. The one integration failure seen on the first run is a pre-existing SSDP multicast flake, filed as #268. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q6d5j3DE1sqf6Gm34gEAAN
…267) The note blamed the Write tool and regex character classes. Neither is the mechanism: every tool argument arrives as a JSON string, so the decoder turns the escape into a real control byte before any tool runs. Write, Edit and Bash all do it — Bash is just the one that rejects the call outright instead of writing the byte. Adds the prevention (double the backslash) alongside the existing repair, and states the range as U+0000-U+001F so the prose no longer needs an inline escape that would itself decode. Hit a third time while editing this very line. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Q6d5j3DE1sqf6Gm34gEAAN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #267.
Five instruction conflicts, each of which could send an agent the wrong way, plus a local gate that was weaker than CI.
a. Issues close on merge
AGENTS.mdtold agents to close the issue "immediately after committing" while also writingcloses #Nand ending in a Draft PR — that closes issues while review is still pending. Now: referencecloses #N, let the merge do it, never hand-close at commit time.b. AGENTS.md no longer duplicates the architecture
Its own §Documentation rules forbid architecture in the file, but §Hub/Player boundary carried ~20 lines lifted from
docs/system-architecture.md— including a second copy of theplayerFilesglob list, which would have drifted. Reduced to seven rule lines; architecture referenced by path.c. CLAUDE.md holds only Claude-unique rules
Its §Workflow was general process, duplicating the
AGENTS.mdchecklist against its own closing rule. Removed;FROM @claude:and the Write-tool control-character gotcha stay.d. Commit rules consolidated
New
AGENTS.md§Branching and commits. Feature branch is the default. Direct-to-mainneeds an explicit ask, and committing or pushing there each need their own — the old wording let the checklist's unconditional "commit in phases" overrideCLAUDE.md's "never automatically commit onmain".e.
pnpm verifymatches CIpnpm verifylint:boundary+ testunitjoblint:boundary+ test + test:integrationA green
verifycould previously still fail CI two ways: full-lint warnings (whichAGENTS.mddemanded separately, three bullets deep) andtest:integration. Full lint subsumes the boundary rules;lint:boundarystays a separate script so CI keeps an isolated boundary signal.Verification
Typecheck, full lint (zero output), 166 frontend + hub unit tests, and integration tests all green locally. The first run hit one integration failure — a pre-existing SSDP multicast flake, ~1 run in 3, unrelated to this change and filed as #268.
Not in this PR: the documentation-drift findings from the same review (stale
/admin/activity/*paths inhub-internals.md, the merge-worker gap insystem-architecture.md). Those get their own issue.🤖 Generated with Claude Code
https://claude.ai/code/session_01Q6d5j3DE1sqf6Gm34gEAAN