fix(cli+framework): fw-4.43.0 / cli-3.45.0 — close #416 and #415 - #417
Merged
Conversation
Two adopter reports, both silent-failure shaped: the failure produced a message that looked normal. #416 — charter refresh-suggest never found telemetry. close writes CHARTER-NN.telemetry.yaml (slug stripped so the name survives a rename); refresh_suggest built <NN-slug>.telemetry.yaml from the Charter's own file stem. The reader never found what the writer wrote, so the refresh heuristic has been inert in EVERY repo with closed Charters since the names diverged. Reproduced on a second repo before touching anything: 27 telemetry files on disk, every Charter reported (missing), chain length 0. What makes it worse than its size: "Chain shorter than 3 closed Charters with telemetry — heuristic not yet meaningful" is indistinguishable from the normal early state, so a seven-Charter chain ran past it unsuspected. Both sides now derive the name from one helper in charter/mod.rs, with a fallback to the legacy name. The regression test asserts what matters — not the string, but that writer and reader derive it from the same place. Also labels the two thresholds (3 = sample size, 6 = trigger value) that were printed as bare numbers and read as contradictory. #415 — duplicate FU ids. THE REPORTED CAUSE DOES NOT REPRODUCE: ids are already assigned per entry, not per AILOG, and have been since before cli-3.41.0. Two real paths, both reproduced: - Parallel branches each computing max+1 against their own copy, then merged. The #391 merge driver renumbers on merge — it was wired in no repo. - Triage pruning a closed entry to a provenance bullet removes its heading, so max() drops and the next drift --apply reissues the number. This one was documented in Sentinel's own registry on 2026-06-04 and went unfixed for two months. Fixes: the high-water mark scans every FU-NNN mention in the body, so a pruned id stays retired; find_entry_unique refuses an ambiguous id and names both entries, replacing first-match resolution in note/set-status/verify/promote/ status; FOLLOWUP-DUPLICATE-ID reports the state as an error, because unlike most registry findings it silently misdirects writes. Docs: the telemetry convention was documented wrong in STRAYMARK.md and QUICK-REFERENCE.md in all three languages — the docs corroborated the reader, not the writer. 967 tests pass. Work in AILOG-2026-08-07-001. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
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
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.
Closes #416. Closes #415.
Two adopter reports with the same shape: the failure produced a message that looked normal.
#416 —
refresh-suggestnever found telemetryThe report was correct in every particular, including which of the two filenames was the right one to keep.
close.rs(writer)CHARTER-NN.telemetry.yaml— slug stripped, so the name survives a renamerefresh_suggest.rs(reader)<NN-slug>.telemetry.yaml— from the Charter's own file stemReproduced on a second repo before touching anything. Sentinel has 27 telemetry files, all canonical, and:
So the refresh heuristic — the mechanism that detects accumulated spec drift across a Charter chain — has been inert in every repo with closed Charters since the names diverged.
Why it outweighs its size:
Chain shorter than 3 closed Charters with telemetry — heuristic not yet meaningfulis indistinguishable from the normal early state. A seven-Charter chain ran past it without anyone suspecting a defect.Fix. One
canonical_telemetry_nameincharter/mod.rs, used by both sides, with a fallback to the legacy name for telemetry older CLIs may have written. The regression test asserts what actually matters — not the string, but that writer and reader derive it from the same place.After, on the same repo:
CHARTER-02.telemetry.yamlresolved, chain length1, rolling mean computed.Also: the two thresholds were printed as bare numbers and read as contradictory. Each line now names what it gates —
3is the sample size needed to evaluate,6is the value the mean must beat.#415 — duplicate FU ids
The reported cause does not reproduce. Ids are already assigned per entry, not per AILOG —
next_n += 1has been inside the inner loop since before cli-3.41.0, and two AILOGs in one pass produceFU-001andFU-002correctly. Suggestion 1 was already implemented.Two real paths, both reproduced:
max + 1against their own copy, then merged — reproduced with the exact reported shape (same id, differentOrigin, differentSource-hash). The followups: the registry is a guaranteed conflict between parallel PRs, and the correct resolution silently reverts closures #391 merge driver renumbers on merge; it was wired in no repo, which is what feat(cli): cli-3.44.0 — install-merge-driver, and correct what skipping it costs #414 addresses.max()drops, and the nextdrift --applyreissues the number. This one was documented in Sentinel's own registry on 2026-06-04 — "prune-to-provenance-bullet hides closed ids … collision reported upstream" — and went unfixed for two months.Fixes.
FU-NNNmention in the registry body, so a pruned id stays retired.find_entry_uniquerefuses an ambiguous id and names both entries, replacing first-match resolution innote/set-status/verify/promote/status.FOLLOWUP-DUPLICATE-IDrule, as an error — unlike most registry findings this one silently misdirects writes, and no reading of a duplicate id is intentional.Docs
The telemetry convention was documented wrong in
STRAYMARK.md(§10, §15) andQUICK-REFERENCE.md— in all three languages. The docs corroborated the reader, not the writer, so an adopter checking the name by hand would have found confirmation of the wrong one. Same correction in thecharter amend --merge-intoguidance the CLI prints.(Correction to my first comment on #416: I said five files including
SPECKIT-CHARTER-BRIDGE.md. It is four — my grep matched a line whereNN-slug.md, which is correctly the Charter file, co-occurs with the word "telemetry".)Verified
cargo test: 967 passed, 0 failed.charter/mod.rs: 4 unit tests including writer/reader agreement.duplicate_fu_ids_test.rs: 5 tests reproducing the reported sequence — the refusednote/set-statusleave the registry byte-identical;validatereports the duplicate; a clean registry stays silent (the rule keys on headings, so a cited id is not a second entry); a pruned id is not reissued.refresh-suggeston Sentinel fixed as above;validatereports no duplicate ids there today (the operator repaired the FU-345 collision by hand before reporting).Known limits
statusnow fails on an ambiguous id where it used to print something. Deliberate: showing one of two entries sharing a number is how an operator concludes the wrong one is the one they are about to write to.followups liststill shows everything.FOLLOWUP-DUPLICATE-IDbeing an error means a repo with existing duplicates starts failingvalidate, including in CI. Intended — the fix is mechanical and the hint says so. A warning would reproduce the original failure, which is that nothing said anything.<NN-slug>.telemetry.yamlon disk. Added because the reporter suggested it and it costs oneexists()call; the test covers resolution order, not a real-world case.🤖 Generated with Claude Code