fix(bin): honor the stated decision key behind a bare corr= prefix - #2679
Open
Kallas95 wants to merge 2 commits into
Open
fix(bin): honor the stated decision key behind a bare corr= prefix#2679Kallas95 wants to merge 2 commits into
Kallas95 wants to merge 2 commits into
Conversation
A bare (no brackets) corr=<hex> correlation token may lead the note ahead
of the stated [key=...] token in a colon-first status line
("blocked: corr=<hex> [key=x] note"), because bin/fm-brief.sh tells
workers to prepend a corr=<id> token to a parent status reply. The fold
read corr=... as the note head, missed the stated key, and opened the
decision under "default"; a later "resolved [key=x]: ..." then closed a
key that was never opened, leaving "default" open forever. The two
decision readers diverged: the OPEN DECISIONS drain showed a decision
still open, while "fm-send --resolve-key x" refused with no open
decision for that key.
Add _fm_note_skip_leading_corr, shared by _fm_key_at_note_head and
status_line_note, that drops exactly one leading corr=<hex> token when
[key= immediately follows it. The stated key is honored in both the open
and close lines, so a decision closed by a later resolved is CLOSED
regardless of either corr, and both readers agree.
The skip is narrow: only a hex-only corr value immediately followed by
[key= is dropped, so a [key=x] mentioned deeper in the note stays prose
and a corr-only note keeps its correlation value. The shared helper also
lets a reserved pending-reply key's vocabulary check reach its
pending-reply...: token when a corr prefix leads the line.
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.
Intent
Reconcilier les deux lecteurs de decisions de firstmate qui se contredisaient sur le journal state/maxisport.status. Le bug: une ligne d ouverture 'blocked: corr= [key=instructions-non-parvenues] ...' place un token de correlation 'corr=' NUE (sans crochets) apres le deux-points, juste devant le token '[key=...]' de la note. Le pli (bin/fm-classify-lib.sh, status_open_decisions / status_open_decisions_incremental via _fm_decision_fold_line -> _fm_decision_key -> _fm_key_at_note_head) lisait 'corr=...' comme tete de note, manquait la cle enoncee, et ouvrait la decision sous 'default'; la ligne de fermeture ulterieure 'resolved [key=instructions-non-parvenues]: ...' fermait alors une cle jamais ouverte (no-op), laissant 'default' ouverte a jamais. D ou la divergence: le pli OPEN DECISIONS du drain (bin/fm-wake-drain.sh) presentait une decision encore ouverte, tandis que 'fm-send --resolve-key instructions-non-parvenues' refusait ('no open decision or blocker with that key'). Les deux lecteurs utilisent en realite le MEME pli; la voie generale a cle de #2490 (state/decision-bindings/) est un registre de second secours pour les decisions transferees au backlog et ne joue aucun role ici.
Semantique voulue: une cle fermee par un resolved ulterieur est FERMEE, quel que soit le corr - la cle est appariee sur le slug seul, la valeur de corr est une metadonnee de correlation sans effet sur l appariment.
Correctif: nouvelle helper _fm_note_skip_leading_corr dans bin/fm-classify-lib.sh, partagee par _fm_key_at_note_head et status_line_note, qui ignore exactement un token de tete 'corr=' (valeur hex-only) QUAND '[key=' suit immediatement, puis cherche le '[key=...]'. Ainsi la cle enoncee est honoree dans les deux lignes (ouverture et fermeture), la resolved ferme la blocked, et les deux lecteurs rendent le meme verdict (fermee). La helper est etroite par construction: un '[key=x]' mentionne plus profond dans la note reste de la prose (ne devient pas une cle), une valeur corr non-hex (ex 'corr=xyz') n est pas reconnue, et une note corr-only (sans [key=]) garde sa valeur de correlation dans le texte affiche. La helper partagée fait aussi que status_line_note depouille le meme prefix corr, si bien que la note colon-first correspond a la forme before-colon et que le controle de vocabulaire des cles reservees (pending-reply-*) atteint son token 'pending-reply...:' meme quand un prefix corr le precede.
Criteres d acceptation: (1) un test de reproduction du defaut existe et passe apres correctif - tests ajoutes dans tests/fm-classify-decision-key.test.sh: test_bare_corr_prefix_then_key_opens_under_stated_key, test_bare_corr_prefix_open_closed_by_resolved (reproduction exacte de la forme maxisport, deux corr differents), test_bare_corr_prefix_key_stays_open_when_unresolved, test_bare_corr_prefix_does_not_swallow_mid_note_prose, test_bare_corr_prefix_reserved_key_vocabulary_passes; (2) sur le journal reel de maxisport, les deux lecteurs rendent le meme verdict 'fermee' (pli vide) - verifie; (3) aucun changement de comportement pour les cles reellement ouvertes - test qui le prouve. La suite fm-classify-decision-key complete passe, ainsi que fm-wake-drain-open-decisions, -cursor, fm-decision-hold-lifecycle, fm-send-resolve-key, fm-pending-reply. shellcheck propre sur le script modifie.
What Changed
_fm_note_skip_leading_corrtobin/fm-classify-lib.sh, shared by_fm_key_at_note_headandstatus_line_note: a colon-first status line likeblocked: corr=<hex> [key=x] notenow drops exactly one leading bare hexcorr=token (tolerating a whitespace run) when[key=immediately follows, so the line opens under the stated key instead ofdefault, a laterresolved [key=x]:actually closes it, and the OPEN DECISIONS drain andfm-send --resolve-keyagree. The skip is narrow: non-hex corr values and[key=...]mentioned deeper in the note stay prose, and the same stripping lets the reserved pending-reply key vocabulary check reach its token.FM_OPEN_DECISIONS_FOLD_VERSIONfrom 4 to 5 so persisted pre-fix cursors carrying a ghostdefaultopen decision are refolded closed under the new semantics.tests/fm-classify-decision-key.test.sh(open under stated key, close regardless of corr, unresolved stays open, mid-note prose and non-hex corr untouched, reserved-key vocabulary, whitespace run before the key) and a v4-cursor ghost-default migration case intests/fm-wake-drain-open-decisions-cursor.test.sh.Risk Assessment
✅ Low: Correctif de bug étroit et bien borné : les deux findings du tour précédent sont réellement appliqués (bump de version du pli à 5 avec invalidation vérifiée des curseurs v4, skip corr tolérant un run d'espaces), couverts par de vrais tests de régression comportementaux qui échoueraient sans le fix, et la passe adversariale n'a trouvé aucun chemin résiduel atteignable vers la divergence des deux lecteurs.
Testing
Validation ciblée complète : suites fm-classify-decision-key (19 ok) et fm-wake-drain-open-decisions (8 ok) vertes, nouveau test de migration du curseur v4 extrait et vert, tests de reproduction vérifiés rouges contre la lib d'avant correctif, et démonstration E2E sur le journal réel de maxisport où les deux lecteurs (drain OPEN DECISIONS et fm-send --resolve-key) divergent avant le correctif puis rendent le même verdict « fermée » (pli vide) après, une clé réellement ouverte restant ouverte et fermable. Réserve : les suites fm-wake-drain-open-decisions-cursor (complète), fm-decision-hold-lifecycle, fm-send-resolve-key et fm-pending-reply sont bloquées localement par le pare-feu utilisateur (il refuse les scripts contenant chmod) et relèvent de la CI distante ; le shellcheck demandé relève de la phase lint. Pas d'artefact visuel : le changement est purement CLI/bibliothèque shell, les transcripts CLI sont la surface utilisateur réelle.
Evidence: E2E journal réel maxisport : les deux lecteurs avant/après correctif (pli fantôme → pli vide, verdict commun « fermée »)
Source: E2E journal réel maxisport : les deux lecteurs avant/après correctif (pli fantôme → pli vide, verdict commun « fermée »)
Evidence: E2E scénarios synthétiques : journal résolu et clé réellement ouverte, avant/après (le drain liste la clé énoncée, fm-send passe la porte de refus)
Source: E2E scénarios synthétiques : journal résolu et clé réellement ouverte, avant/après (le drain liste la clé énoncée, fm-send passe la porte de refus)
Evidence: Tests de reproduction : échec contre la lib de base, suites vertes après correctif, migration curseur v4
Source: Tests de reproduction : échec contre la lib de base, suites vertes après correctif, migration curseur v4
Evidence: Verdict pivot du pli brut sur le journal réel
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
bin/fm-classify-lib.sh:483- FM_OPEN_DECISIONS_FOLD_VERSION reste a 4 alors que la semantique de _fm_decision_fold_line change (une ligne 'blocked: corr=<hex> [key=x] ...' se plie desormais sous 'x' au lieu de 'default'). La doc du fichier (bin/fm-classify-lib.sh:443-445) exige un bump a chaque changement de semantique du pli, et les deux changements precedents l'ont fait (2->3 pour fix(bin): honor decision keys after the verb colon #2202, le fix analogue colon-first; 3->4 pour fix: parse decision verbs before status metadata tags #2280). Sans bump, un sidecar de curseur ecrit avant le fix (state/.<task>.open-decisions-cursor, version=4) reste valide: le pli incremental (status_open_decisions_incremental ne relit jamais les octets consommes, lignes 592-599; scan_open_decisions_incremental alimente la section OPEN DECISIONS de bin/fm-wake-drain.sh:137) continue de presenter le fantome 'default' ouvert a jamais, tandis que le pli complet (fm-send --resolve-key, bin/fm-send.sh:424) rend 'ferme' et refuse tout append de resolution - exactement la divergence des deux lecteurs que ce fix pretend eliminer durablement, desormais sans voie de purge supportee. Correctif: bumper FM_OPEN_DECISIONS_FOLD_VERSION a 5 pour invalider les curseurs pre-fix et rejouer le pli depuis l'octet 0 (idealement avec un test de regression qui pre-seed un curseur v4 contenant le fantome 'default' et verifie que le pli incremental le reconstruit).bin/fm-classify-lib.sh:230- Le motif "$first"[[:space:]][key=* n'accepte qu'UN caractere d'espacement entre le token corr et '[key=': 'blocked: corr=9c91ae0ff46cdca1 [key=x] note' (deux espaces, ou tab+espace) n'est pas skippe, la ligne rouvre 'default' et un 'resolved [key=x]:' ulterieur redevient un no-op - la panne d'origine a un espace pres. Le chemin sans corr tolere pourtant un run d'espaces (trim de tete en bin/fm-classify-lib.sh:247) et le commentaire de la helper dit 'when "[key=" is the very next token'. Correctif: apres validation hex du token, remplacer le case par strip du token (rest=${note#"$first"}), trim du run d'espaces (rest=${rest#"${rest%%[![:space:]]}"}) puis test de prefixe 'case "$rest" in [key=)' avant d'affecter note=$rest.🔧 Fix: bump fold version to 5, widen corr whitespace skip
✅ Re-checked - no issues remain.
tests/fm-send-resolve-key.test.sh- Quatre suites citées dans les critères d'acceptation n'ont pas pu être exécutées localement : le hook pare-feu de la machine (~/.claude/hooks/pre-tool-use-firewall.mjs) refuse d'exécuter tout script contenant 'chmod', or fm-wake-drain-open-decisions-cursor (suite complète), fm-decision-hold-lifecycle, fm-send-resolve-key et fm-pending-reply créent leurs shims fakebin avec 'chmod +x'. Compensations effectuées sans contourner le pare-feu : le seul nouveau test du curseur (test_pre_fix_v4_cursor_ghost_default_is_refolded_closed) a été extrait dans un pilote sans chmod et passe, et le comportement de fm-send --resolve-key a été exercé en E2E sur le journal réel. La CI distante doit confirmer ces suites complètes ; l'auteur peut aussi vouloir ajuster l'allowlist du hook pour que les suites du dépôt redeviennent exécutables localement.bash tests/fm-classify-decision-key.test.sh- 19 ok, dont les 6 nouveaux tests bare-corr (critère 1)bash tests/fm-wake-drain-open-decisions.test.sh- 8 ok, câblage réel du drainExtraction sans chmod detest_pre_fix_v4_cursor_ghost_default_is_refolded_closed(tests/fm-wake-drain-open-decisions-cursor.test.sh) exécutée contre le vrai drain - ok, un curseur v4 portant le fantôme 'default' est replié ferméRouge-avant/vert-après : le fichier de test du worktree exécuté contre la lib de base 1cb900c échoue avec le défaut exact ('default' + corr en tête de note)E2E journal réel : copie de /Users/max/Documents/projects/firstmate/state/maxisport.status rejouée dansfm-wake-drain.shetfm-send maxisport --resolve-key instructions-non-parvenuesavec l'arbre de base puis l'arbre corrigé - divergence reproduite avant, verdict commun « fermée » (pli vide) après (critère 2)E2E clé réellement ouverte (journal sans resolved) : le drain corrigé liste[key=instructions-non-parvenues]et fm-send passe la porte de refus, n'échouant qu'à la livraison tmux du bac à sable (critère 3)git status --porcelainfinal vide - aucun résidu de test dans le worktree✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.