ci: add a type: chore label, and record the uvx MCP launch - #15
Conversation
The taxonomy had type: refactor, docs, ci and security but nothing covering test-only or tooling work, while the branch naming rule already uses `chore` as one of its three kinds. Filing a ticket for "add the missing tests to paths.go" had no correct label, which surfaced the gap. type: refactor is "internal change with no behaviour change" and adding a missing test is not a refactor, so this is a distinct type rather than a stretch of an existing one. Closes #14 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JoottunAtish
left a comment
There was a problem hiding this comment.
Review of my own PR, so this is a comment rather than an approval: GitHub does not allow self-approval, which is the exact situation the RepositoryAdmin bypass in the ruleset exists for.
One blocking issue, one should-fix, one suggestion. All eight checks are green and I verified the label sync independently, so the label change itself is sound. The problem is what came along with it.
blocking: this PR contains an undeclared, unrelated change
The diff is two files, not one. .mcp.json has been rewritten from direct binary invocation to uvx:
- "command": "jcodemunch-mcp",
- "args": ["serve", "--transport", "stdio", "--watcher"],
+ "command": "uvx",
+ "args": ["jcodemunch-mcp"]The title says "add a type: chore label", the description talks only about labels, and the linked ticket #14 is scoped to labels.yml. A reviewer approving on the description would not know the MCP configuration changed.
This violates the one-logical-change rule in the github-workflow skill, and it matters more than tidiness here: the two changes have different blast radii. The label addition is additive and reversible. The MCP change alters how this repository's tooling launches for every contributor, and it cannot be reverted by removing a label.
On its merits the uvx form is defensible and probably correct, because the direct-binary form did not reliably connect while the uvx form did. But two things in it are behavioural changes nobody declared:
--watcheris dropped from jcodemunch. That flag enabled filesystem watching and auto-reindexing. Thecode-navigationskill claims hooks handle reindexing on Edit and Write, so this is probably fine, but it is a capability removal and it should be a stated decision rather than a side effect.uvxresolves the package at launch, so the version that runs is whateveruvxpicks, not the version installed on the machine. That is a supply chain posture change in a repository that just spent a whole ticket pinning GitHub Actions to commit SHAs for exactly that reason.
Requested fix: split it. Either drop .mcp.json from this branch and file it as its own chore ticket, or keep it and rewrite the title, description and ticket to cover both, explicitly noting the --watcher removal and the uvx resolution behaviour. The first is cleaner.
should fix: this PR makes PROGRESS.md wrong in two places
PROGRESS.md on main states the label count twice:
line 33: | Label taxonomy | 32 labels defined in `.github/labels.yml`, ... |
line 92: - 32 issue labels defined in `.github/labels.yml` and applied by
The PR head has 33. I verified by counting ^- name: on the branch.
Small, but it is the kind of drift that makes a progress document stop being trusted, and the implementation skill's Definition of Done asks for PROGRESS.md to be updated when a change alters what exists. Either correct both numbers or replace them with something that cannot go stale, for example "the label taxonomy is defined in .github/labels.yml" with no count.
suggestion: five GitHub default labels shadow the taxonomy
Not caused by this PR, but directly adjacent to it and worth catching while we are here. Comparing the declared set against the live repository:
on GitHub but NOT in labels.yml: bug documentation enhancement invalid question
Those are GitHub's repository defaults. bug, documentation and enhancement duplicate type: bug, type: docs and type: feature, and they actively undermine the "exactly one type: label per issue" rule, because sooner or later somebody applies bare bug and the taxonomy quietly stops meaning anything.
sync-labels.sh deliberately never deletes, which is the right default, so these will linger until removed by hand. Worth a follow-up ticket to delete the five and note in the script's header that the repository defaults were pruned once on purpose.
What I verified rather than assumed
- 33 labels on the PR head, up from 32
d4c5f9appears exactly once, so no colour collision. The repeated colours in the file (1d76db,5319e7,b60205) are the intentionalarea:,platform:and severity families, not accidents../scripts/sync-labels.shapplied cleanly: 33 created, 0 failed, and the awk parser handled the new entry- Every declared label exists on GitHub, so the sync is complete
type: choreapplied to #13 and it stuck- All eight checks green, including the branch-name gate on
chore/issue-14
Safety review: not applicable to the label change. The .mcp.json change touches local tooling launch only and cannot reach a learner's machine, but see the uvx version-resolution note above.
Follow-up to the review on #15, which found two problems with that branch. The branch carried an undeclared change to .mcp.json alongside the label addition. The change itself is correct and intentional: both servers now launch through uvx because the direct-binary form did not connect reliably. So rather than reverting it, this records it where the next person will look, in the code-navigation skill, together with the two consequences that are easy to miss: uvx resolves the package version at launch, and the jcodemunch --watcher flag is no longer passed, so reindexing depends on the PostToolUse hooks. That section also now covers what to do when a server does not connect, since editing .mcp.json mid-session does nothing until the window is reloaded, and the fact that indexing docs here needs include_dot_dirs for .claude, or the skills that hold the project's rules are silently absent from the index. PROGRESS.md quoted "32 labels" in two places, which adding type: chore made wrong. Both now point at .github/labels.yml as the source of truth instead of repeating a number, because a count in prose goes stale the first time somebody adds a label. That is the second time this file has drifted, so removing the number is better than incrementing it. Also ignores __pycache__ and friends. Writing this commit staged two .pyc files from the pytest suite, which .gitignore did not cover because Python only arrived in this repository with the CI gate checker. Closes #14 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
All three review findings addressed in blocking, the undeclared should fix, the stale label count. Both occurrences in suggestion, the shadow labels. Filed as #17, with the zero-usage check already done for all five so the deletion is provably safe. One thing the review did not catch, found while committing: nothing ignored Diff is now five files: the label, the MCP config, the skill, |
What this changes
Two related repository-configuration changes, both now declared. Closes #14.
type: choreto the label taxonomy..mcp.jsonlaunch change that was already on this branch.Type
1. The label
The taxonomy had no label for test-only or tooling work, so #13 ("add the missing tests to
paths.go") had nowhere correct to sit.type: refactormeans "internal change with no behaviour change", and adding a missing test is not that.Meanwhile the branch naming rule already used
choreas one of its three kinds, so the word was in the vocabulary with no label behind it.2. The MCP launch change
This was undeclared in the first version of this PR. The review caught it, and this is the fix.
.mcp.jsonlaunches both servers throughuvxrather than by calling the installed binary directly. That is intentional and it stays: the direct-binary form did not connect reliably, and theuvxform does.Rather than reverting it, the reasoning is now recorded in the
code-navigationskill where the next person will actually look, along with the two consequences that are easy to miss:uvxresolves the package at launch, so the version that runs is whatever it picks rather than whatever is installed.--watcheris no longer passed to jcodemunch, so there is no filesystem watcher. Reindexing relies on thePostToolUsehooks, which fire onEditandWrite. If the index looks stale, callregister_editrather than waiting for a watcher that is not running.That skill section also gained the operational knowledge this session cost us to learn: editing
.mcp.jsonmid-session connects nothing until the window is reloaded, and indexing docs here needsinclude_dot_dirs: [".claude"]or the skills holding the project's rules are silently absent from the index.Also in here
PROGRESS.mdquoted "32 labels" in two places, which adding a label made wrong. Both now point at.github/labels.ymlas the source of truth instead of repeating a number. Second time that line has drifted, so removing the count beats incrementing it..gitignorenow covers__pycache__and*.pyc. Writing this commit staged two bytecode files from the pytest suite, because Python only arrived in this repository with the CI gate checker and nothing ignored its caches.Checklist
./scripts/check-punctuation.shpasses./scripts/check-links.shpasses./scripts/sync-labels.shapplied cleanly, 0 failed.pycor__pycache__path is trackedPlatforms exercised
Notes for the reviewer
sync-labels.shnever deletes labels absent from the file, deliberately, because deleting one silently removes it from every issue that carried it. So the label change is additive only.Separately, the review noted that five GitHub default labels (
bug,documentation,enhancement,invalid,question) shadow thetype:taxonomy and undermine the one-type:-label rule. Not fixed here, since it is a deletion rather than an addition and deserves its own change. Tracked as a follow-up.