fix: don't split the ledger — external billing is opt-in (v0.72.0) - #440
Merged
Conversation
#439 handed /timer and /billing to @profullstack/timer and @profullstack/billing the moment those packages appeared on PATH. That was wrong, and the pit's own test caught it: only half this layer has an outside home. /client, /rate, /payments and /team stay in moshcode — the rails and the permission model are ours, and /client's freeform dotted fields have no shape in the package's typed client model, so moving it would lose data rather than relocate it. Handing over the other half automatically splits one person's records across two stores: /client create "Acme Inc" → ~/.moshcode/business.json /rate set acme-inc $100/hour → ~/.moshcode/business.json /billing acme-inc → reads the package ledger: "no client acme-inc" That is not a missing feature, it is somebody's invoice failing to exist. And it only happens on a machine that installed the CLIs, so CI — which has not — stayed green while every developer box that took the install went red. That asymmetry is why it shipped. So the hand-over is now opt-in: MOSHCODE_EXTERNAL_BILLING=1, after `billing import` has moved the records across. The install tip only appears to somebody who has the package installed and has not switched it on, and it names the import, because switching over without it is the same split by hand. Two guards, both verified to fail when the old behaviour is put back: a unit test that installed is not the same as chosen, and a pit test that puts a fake `timer` on PATH and checks the pit still wrote its own ledger. The second is the one that would have caught this originally — the first pit test did, on a machine with the tools installed, which no CI run has. When the whole layer has an outside home, this becomes the default. Version bumped to 0.72.0 for the release that carries #439 and this fix.
ThreatCrush Security Scan0 finding(s) in the 5 file(s) this pull request changes. Nothing in the files this pull request changes. 65 pre-existing finding(s) elsewhere in the repository — **HIGH/CRITICAL**: 5 | **MEDIUM**: 54 | **LOW**: 6Not introduced by this pull request. The full set is in the Security tab.
…and 45 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
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.
#439 handed
/timerand/billingto the standalone CLIs the moment those packages appeared on PATH. That was wrong, and the pit's own test caught it as soon as I installed them on a real box.The bug
Only half this layer has an outside home.
/client,/rate,/paymentsand/teamstay in moshcode — the rails and the permission model are ours, and/client's freeform dotted fields have no shape in the package's typed client model, so moving it would lose data rather than relocate it.Handing over the other half automatically splits one person's records across two stores:
That is not a missing feature, it is somebody's invoice failing to exist.
And it only breaks on a machine that installed the CLIs — so CI, which has not, stayed green while every developer box that took the install went red. That asymmetry is why it got merged.
The fix
The hand-over is opt-in:
MOSHCODE_BUILTIN_BILLINGis gone — with the default the other way round it had nothing to do. The install tip now appears only to somebody who has the package installed and has not switched it on, and it names the import, because switching over without it is the same split by hand.Two guards
Both verified to fail when the old behaviour is put back:
timeron PATH and checks the pit still wrote its own ledgerThe second is the one that would have caught this originally. The existing pit test did catch it — but only on a machine with the tools installed, which no CI run is.
When the whole layer has an outside home, delegation becomes the default.
Release
package.json→ 0.72.0, so this and #439 ship together. Nothing has reached an installed moshcode yet: the newest release isv0.71.0, cut from880d565, the commit before #439 merged.Suite is 2275 passing, 0 failing — run on a box where both CLIs are installed, which is the case that was broken.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KVUZsx4WBZK5rnJJFGgwF5