fix(app-repo): count an idempotent skill re-install as applied, not "unaccounted for" - #90
Merged
Merged
Conversation
…unaccounted for"
A regression I introduced with hermiq's idempotent installer, caught by reading
the seam rather than by any failing test.
The applier read only `installed` from hermiq's response. Once hermiq stopped
duplicating skills, a re-install of a bundle already present reports
installed: 0 updated: 0 unchanged: 94
so the applier accounted for 0 of 94 declared items and absorbed the shortfall as
"not-accounted-for-by-source" — a loud failure banner on a perfectly good run,
and precisely the kind of false alarm that trains people to ignore the report.
"Present as intended" is installed + updated + unchanged. The source's own
breakdown is now carried through unflattened in `sourceCounts`, so a first install
and a no-op re-run stay distinguishable instead of collapsing into one total.
Mutation-checked: reverting to `created: $installed` turns the new test red
(0 vs 2).
The skills channel moved into its own SkillChannelDelegate — talking to another
app across an optional-dependency boundary is a different responsibility from
applying OpenRegister channels, and phpmd flagged the applier at 51 > 50 when the
mapping was added inline. A real split, not a suppression; the applier's tests
compose the REAL delegate so the degradation and count assertions keep biting.
754 tests OK, phpstan 0, phpcs 0, phpmd 0.
Contributor
Quality Report — ConductionNL/openbuild @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ❌ | ||||
| stylelint | ❌ | ||||
| composer | ✅ | ✅ 100/100 | |||
| npm | ❌ | ❌ | |||
| PHPUnit | ⏭️ | ||||
| Newman | ⏭️ | ||||
| Playwright | ⏭️ |
Quality workflow — 2026-08-02 11:22 UTC
Download the full PDF report from the workflow artifacts.
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.
A regression I introduced with hermiq's idempotent installer (ConductionNL/hermiq#112), caught by reading the seam rather than by any failing test.
The defect
The applier read only
installedfrom hermiq's response. Once hermiq stopped duplicating skills, a re-install of a bundle already present reports:So the applier accounted for 0 of 94 declared items and absorbed the shortfall as
not-accounted-for-by-source— a loud failure banner on a perfectly good run, and precisely the kind of false alarm that trains people to ignore the report.The fix
"Present as intended" is
installed + updated + unchanged. The source's own breakdown is carried through unflattened insourceCounts, so a first install and a no-op re-run stay distinguishable rather than collapsing into one total.Mutation-checked: reverting to
created: $installedturns the new test red (0 vs 2).Also: the skills channel moved to its own delegate
Talking to another app across an optional-dependency boundary is a different responsibility from applying OpenRegister channels — and phpmd flagged the applier at 51 > 50 when the mapping was added inline. A real split, not a suppression.
The applier's tests compose the real
SkillChannelDelegaterather than a mock, so the degradation and count assertions keep biting.754 tests OK · phpstan 0 · phpcs 0 · phpmd 0.