Repository navigation
fix(sync-project): ignore the toolkit's own wiring block in the baseline-only skills warning (#382) - #398
Merged
Conversation
…block or a missing heading (#382) Strip mxtk:wiring block before the grep; warn only when the Integration heading exists. Test T4 flipped, T7-T9 added. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`grep && bad || ok` counted a failure as both FAIL and ok (bad returns 1). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
…398) tests/wave2/test-pipefail-grep-q.sh flagged the new check under pipefail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MendixMau
added a commit
that referenced
this pull request
Oct 15, 2026
…ot-installed so sync stops warning (#381) (#399) Reviewed head: a6b5f46 Verdict: CHANGES (the CHANGELOG line is in the wrong place; code and test are good) PR #399 fix(install-manifest): coverage-check-all.sh and ts-sync.sh move to MXTK_PROJECT_BIN_NOINSTALL (#381). Impact rows (docs/RELEASE-PROCESS.md 4a) - Process: confirmed. A warning is removed and a test added. No human step. - Dependencies: confirmed. bin/lib/install-manifest.sh is shared and sourced by sync-project.sh and init-project.sh. Also CHANGELOG.md and the new test. No new artifact read. - Context cost: confirmed none. install-manifest.sh, CHANGELOG.md and the test are not rows in skill-routing.tsv. ts-sync.sh does have a row (experimental), but that file is not touched. - Behaviour: confirmed n/a (script only). - Runtime: confirmed n/a. The same directory scan as today. 5 of 5 rows confirmed. Correctness - tests/wave2/test-manifest-clean-checkout.sh: on head, 1 passed, 0 failed. Pointed at an origin/master checkout it fails, listing coverage-check-all.sh and ts-sync.sh, which is the correct red/green. - The stated reasons hold. project-bin/review-module.sh:99-105 `_tool` looks in $ROOT/bin, $BIN, then $MXTK_ROOT/bin and project-bin, and MXTK_ROOT is resolved from CLAUDE.local.md (:90-96). The ts-sync.sh header says SUPERSEDED (line 7) and UNPROVEN (line 12). Neither file was ever installed, so no project loses anything. - Side effect, intended: sync-project.sh:276 now refuses an explicit request to install either file, naming NOINSTALL. - BUG (blocking), CHANGELOG.md:10. This is the same defect as #398. The bullet is spliced into the "Release cycle" preamble inside the `## Unreleased` backtick span, not under the heading at line 22. bin/cut-release.sh:47 will not count it, and the preamble paragraph is broken. Fix: restore line 10 as on master and add the bullet as the first item under `## Unreleased`. - Nit: the trailing comment on install-manifest.sh:129 is now one very long line covering three reasons. Consider one comment line per file. Risk - Every sync and init sources the manifest, so the first-line warning disappears for every adopter. A future unnamed project-bin file now fails this wave2 test instead of only warning in the field. --- re-review --- Reviewed head: 2ed20a3 Verdict: APPROVE Re-review of #399 CHANGES finding. - BUG resolved: git diff origin/master...HEAD -- CHANGELOG.md is one hunk, +1 line, the fix(install-manifest) bullet as first item under "## Unreleased". Release-cycle paragraph untouched. - bin/check-pr-discipline.sh origin/master: clean. - Only CHANGELOG changed in the fix-up; the long comment nit on install-manifest.sh:129 is optional and not blocking.
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
1 participant
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.
Direct lane
What changed and why (one paragraph): sync-project.sh grepped all of CLAUDE.md for two baseline-only skill paths, so the toolkit-written mxtk:wiring block triggered the warning, and with no Integration heading it reported a section that does not exist. It now strips the wiring block before the grep and warns only when the Integration heading exists. Closes #382.
Impact
Field evidence — reported by @joshuamoesa from a field project at toolkit d16f578; reproduced on master 354669e by running the check (see linked issue).
tests/wave2/(see below)CHANGELOG.mdline appended, crediting the reporterbranch night/fix-382, commit 3759d1b. Tests: tests/wave2/test-sync-dup-heading-exit.sh, 9 passed 0 failed (T4 flipped, T7-T9 new: wiring-only silent, block plus heading still warns). Base-run of the new cases not executed. Leak guard and private-citations check clean (LEAKGUARD_ALLOW_NO_DENYLIST=1).
🤖 Generated with Claude Code