Skip to content

fix(install-manifest): name coverage-check-all.sh and ts-sync.sh as not-installed so sync stops warning (#381) - #399

Merged
MendixMau merged 3 commits into
masterfrom
night/fix-381
Oct 15, 2026
Merged

MendixMau merged 3 commits into
masterfrom
night/fix-381

Conversation

@MendixMau

Copy link
Copy Markdown
Owner

Direct lane

What changed and why (one paragraph): coverage-check-all.sh and ts-sync.sh were in neither MXTK_PROJECT_BIN list, so every sync and init warned. Both move into MXTK_PROJECT_BIN_NOINSTALL with a reason (coverage-check-all is resolved from the toolkit root by review-module.sh; ts-sync is marked superseded). A new clean-checkout test fails on any unnamed project-bin file. Maintainer call flagged: if projects should get a copy of coverage-check-all.sh, it is a one-word move to MXTK_PROJECT_BIN. Closes #381.

Impact

Row Answer
Process none: no new human step or gate; a warning is removed, a test is added.
Dependencies touches bin/lib/install-manifest.sh, CHANGELOG.md, new tests/wave2/test-manifest-clean-checkout.sh; no new artifact read.
Context cost none: no routed or always-loaded file touched.
Behaviour n/a: script only.
Runtime n/a: nothing on a hot path (one directory scan, already run today).

Field evidence — reported by @joshuamoesa from a field project at toolkit d16f578; reproduced on master 354669e by running the check (see linked issue).

  • No client data anywhere in the diff (leak guard + private-citations clean)
  • Size cap: within 10 files / 400 lines
  • Test tier: T1, scoped test in tests/wave2/ (see below)
  • CHANGELOG.md line appended, crediting the reporter

branch night/fix-381, commit a6b5f46. Tests: new tests/wave2/test-manifest-clean-checkout.sh, 1 passed 0 failed on the branch; on master it fails (lists both files). That test fails, not warns, on any unnamed file, as the issue proposed. Leak guard and private-citations clean.

🤖 Generated with Claude Code

MendixMau and others added 2 commits October 15, 2026 22:32
…OINSTALL; test fails on unnamed files (#381)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MendixMau added a commit that referenced this pull request Oct 15, 2026
…ine-only skills warning (#382) (#398)

Reviewed head: 3759d1b
Verdict: CHANGES (the CHANGELOG line is in the wrong place; also fix the test's && bad || ok pattern)

PR #398 fix(sync-project): ignore the toolkit's wiring block in the baseline-only skills warning (#382).

Impact rows (docs/RELEASE-PROCESS.md 4a)
- Process: confirmed. One warning removed. No step or gate.
- Dependencies: confirmed. bin/sync-project.sh, the wave2 test and CHANGELOG.md. It reads only the project's CLAUDE.md, as before.
- Context cost: confirmed none. None of the three files is a row in bin/lib/skill-routing.tsv.
- Behaviour: confirmed n/a (script only).
- Runtime: confirmed n/a. One awk plus greps over CLAUDE.md, once per sync.
5 of 5 rows confirmed.

Correctness
- The fix (bin/sync-project.sh:1066-1080) is sound. The wiring block is stripped with an awk range,
  the heading is searched in the stripped body, and the word count is taken from that body.
  When CLAUDE.md is missing, _dup_from_line stays empty, so set -u is safe.
- Scoped test on head: 9 passed, 0 failed. Against origin/master's sync-project.sh, T4 and T8 fail
  as intended, which proves the regression test catches the bug.
- BUG 1 (blocking), CHANGELOG.md:10. The entry is spliced into the "Release cycle" preamble,
  inside the backtick span `## Unreleased ...`. It is not under the `## Unreleased` heading
  (line 22). The preamble paragraph is now broken, and bin/cut-release.sh:47 only counts lines
  under `^## Unreleased$`, so this entry would never reach a release. check-pr-discipline.sh does
  not catch this. Fix: restore line 10 as on master and put the `- fix(sync-project): ...`
  bullet as the first item under `## Unreleased`.
- BUG 2 (minor), tests/wave2/test-sync-dup-heading-exit.sh:51 and :69. `grep && bad ... || ok`
  reports both FAIL and ok on a failure, because bad() (line 28) returns 1 when there is no
  detail argument. On master the run printed "9 passed, 2 failed". It still exits non-zero, but
  the counts are wrong. Fix: `if printf ... | grep -q ...; then bad "..."; else ok "..."; fi`,
  or end bad() with `return 0`.

Risk
- This runs on every sync of every project. The change only narrows when a warning appears and
  cannot fail the sync: every grep is guarded with `|| true` or used in an if. A CLAUDE.md with a
  start marker but no end marker would hide the rest of the file from this warning only. That is acceptable.
- #399 edits the same CHANGELOG line, so the two PRs conflict there.

--- re-review ---
Reviewed head: 0528c11
Verdict: APPROVE

Re-review of #398 CHANGES findings.
- BUG 1 resolved: git diff origin/master...HEAD -- CHANGELOG.md is one hunk, +1 line, the fix(sync-project) bullet as first item under "## Unreleased". Release-cycle paragraph untouched (identical to master).
- BUG 2 resolved: T4 and T8 use if/else. Test run on head: 9 passed, 0 failed, exit 0.
- bin/check-pr-discipline.sh origin/master: clean.
- Nothing new broke: diffstat is 3 files (CHANGELOG, sync-project.sh, the test).

--- delta review ---
Reviewed head: 79174ed
Verdict: APPROVE

Diff is one line in bin/sync-project.sh (1 insertion, 1 deletion); nothing else changed.
Equivalent: printf '%s\n' "$x" and <<<"$x" both feed $x plus one trailing newline, so an empty body is "\n" in both and grep exits 1 either way.
grep exit status is unchanged (0 match, 1 no match) and is tested by `if`, so set -e is not triggered. With no pipe, pipefail is moot and there is no SIGPIPE race, which was the lint concern.
Bash 3.2.57 (macOS default) supports here-strings; `--` before the pattern is accepted by BSD and GNU grep. The pattern is unchanged.
The following line (the printf | grep -n ... || true pipeline) is untouched and still guarded.
Local evidence (pipefail lint 3/0, dup-heading-exit 9/0) is consistent with this reading.
@MendixMau
MendixMau merged commit 99ef8f5 into master Oct 15, 2026
2 checks passed
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.

install-manifest.sh warns on every sync: coverage-check-all.sh and ts-sync.sh are in neither MXTK_PROJECT_BIN list

1 participant