docs: pin Herdr alt-screen scrollback bounds and capture viewport stability - #2396
Open
Kallas95 wants to merge 3 commits into
Open
docs: pin Herdr alt-screen scrollback bounds and capture viewport stability#2396Kallas95 wants to merge 3 commits into
Kallas95 wants to merge 3 commits into
Conversation
Document that alternative-screen harnesses, including Claude Code, expose no scrollback history to Herdr, so any capture of such a pane is bounded to the visible window regardless of the requested --lines. Add a viewport-stability non-regression test to the real-herdr smoke suite verifying that no firstmate capture moves the pane viewport.
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
Document that alternative-screen harnesses, including Claude Code, expose no scrollback history to Herdr, so any capture of such a pane is bounded to the visible window regardless of the requested --lines. Place this knowledge in docs/herdr-backend.md near the existing capture/peek sections (the 'Current transport behavior' paragraph about pane read --lines N and the 200-line floor), without duplicating what the doc already says. Add a non-regression test 'a firstmate capture never moves the pane viewport' to tests/fm-backend-herdr-smoke.test.sh, following the exact recipe in section 8 of the scout report data/fm-herdr-scroll-scout/report.md (the source of truth): seed real scrollback with seq 1 500, read .result.pane.scroll before and after the firstmate capture functions, and fail if the viewport moved. The test must pass locally and would fail if a capture moved the viewport. Follow repo style: one sentence per line in the docs, plain dash, shellcheck-clean test. Stay bounded: no doc refactor and no additional tests.
What Changed
docs/herdr-backend.mdnow states, next to the existing capture/transport section, that alternative-screen harnesses (including Claude Code) expose no scrollback history to Herdr, so any capture of such a pane is bounded to the visible window regardless of the requested--lines.tests/fm-backend-herdr-smoke.test.shgains a viewport-stability check: it creates a throwaway pane, seeds real scrollback withseq 1 500, records.result.pane.scrollbefore and after running the firstmate capture helpers (capture,capture_ansi,composer_state), and fails if the pane's scroll position changed.Risk Assessment
✅ Low: Changement purement additif (2 lignes de doc + un bloc de smoke test auto-isolé) : tous les critères de l'intent sont vérifiés en source, les fixes des rounds précédents sont matériellement en place et corrects, et le nouveau code est shellcheck-clean.
Testing
Ran the full real-herdr smoke suite (the smallest unit containing the new check, which depends on the suite's isolated-session setup): all checks pass including the new viewport-stability one, with real seeded scrollback confirmed via .result.pane.scroll. Then demonstrated the test's failure mode end-to-end by fault injection: attaching a real herdr client and scrolling the pane in copy-mode between the before/after reads moved offset_from_bottom from 0 to 29 and made the test's exact assertion fire, so the test provably fails when a capture moves the viewport. Also verified the doc sentences sit in the intended 'Current transport behavior' spot without duplication. No browser/GUI surface exists for this docs+shell-test change; the end-user surfaces are the doc text and CLI test output, captured as text artifacts including the herdr client's terminal screen during the injected scroll. Shellcheck was not run here (lint phase owns it). No transient artifacts left in the worktree.
Evidence: Smoke test transcript (all pass, incl. new viewport-stability check)
ok - real herdr: current_path reads the pane's live cwd ok - real herdr: a firstmate capture never moves the pane viewport (no scroll for alternative-screen harnesses) ok - real herdr: kill removes the pane and is idempotent/best-effortEvidence: Fault-injection transcript: assertion fires when the viewport moves
scroll_before={"max_offset_from_bottom":481,"offset_from_bottom":0,"viewport_rows":23} scroll_after={"max_offset_from_bottom":473,"offset_from_bottom":29,"viewport_rows":31} not ok - a firstmate capture moved the pane viewport: {...offset_from_bottom:0...} -> {...offset_from_bottom:29...} DEMONSTRATED: the suite's viewport-stability assertion FAILS when the viewport movesEvidence: Herdr client screen during the injected scroll (copy-mode, pane scrolled to lines 443-472)
Evidence: Fault-injection script (reproducible demonstration)
Evidence: Doc excerpt: new sentences in 'Current transport behavior' after the 200-line-floor paragraph
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed (2) ✅
tests/fm-backend-herdr-smoke.test.sh:308- Le test de stabilité du viewport ne valide jamais que scroll_before est non vide et non null..result.pane.scrolln'est référencé nulle part ailleurs dans le dépôt ; siherdr pane getéchoue ou si le champ disparaît/est renommé dans une version future de Herdr, jq produitnull(ou vide) avant ET après, la comparaison passe à vide et le test ne peut plus jamais échouer — ce qui contredit silencieusement sa raison d'être (« would fail if a capture moved the viewport »). Ajouter un garde-fou du type{ [ -n "$scroll_before" ] && [ "$scroll_before" != null ]; } || fail "could not read .result.pane.scroll"avant les captures.tests/fm-backend-herdr-smoke.test.sh:309- Les codes de retour des trois invocations de capture sont jetés (>/dev/nullsans|| fail).fm_backend_herdr_captureetfm_backend_herdr_capture_ansiretournent 1 en cas d'échec CLI (bin/backends/herdr.sh:2596,2606) ; si une capture échoue, le scroll est trivialement inchangé et le test passe sans avoir exercé le comportement épinglé. Toutes les autres captures de cette suite sont gardées par|| fail— appliquer le même garde ici (composer_state retourne toujours 0 et n'en a pas besoin, mais son verdict pourrait aussi être vérifié différent deunknown).🔧 Fix: guard scroll read and capture failures in viewport test
2 issues (1 warning, 1 info) still open:
tests/fm-backend-herdr-smoke.test.sh:316- L'assertion[ "$cs" != unknown ]ajoutée par le round de fix traite le verdictunknowncomme un échec de capture, alors que c'est le verdict conçu et légitime du classificateur pour un écran sans composer : la règle stricte de bin/fm-composer-lib.sh (sélection cursorless, lignes 1261-1263, et le catalogue de formes) dit qu'un glyphe de prompt shell (>$%#) n'est jamais une preuve de conteneur. Le pane jetable du test héberge un shell brut affichant la sortie deseq 1 500; sur toute machine avec herdr installé et un prompt par défaut, composer_state retourneunknownet le test échoue systématiquement avec un message trompeur (« could not capture » alors que la capture ANSI a réussi). Il ne passe sur la machine de l'auteur que parce que son prompt starship❯coïncide avec le glyphe composer bare de claude. Correction : supprimer l'assertion et revenir à la forme du commit initialfm_backend_herdr_composer_state "$SCROLL_TARGET" >/dev/null— le but de cet appel est d'exercer le chemin de capture pour la stabilité du viewport, et le succès de la capture est déjà garanti par les deux appels précédents gardés par|| failqui utilisent les mêmes primitives (capture et capture_ansi).tests/fm-backend-herdr-smoke.test.sh:309- Les deux findings du round 1 sont matériellement corrigés : le garde-fou non-vide/non-null sur scroll_before (lignes 309-310) élimine le passage à vide (un échec du pane get d'après ne peut plus produire de faux succès puisque scroll_before est garanti non-null), et les deux captures sont gardées par|| fail(lignes 311-314). Seule la déclinaison composer_state du second fix introduit le défaut signalé séparément.🔧 Fix: drop machine-dependent composer_state assertion in viewport test
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-backend-herdr-smoke.test.sh — full real-herdr smoke suite (isolated fm-lab session, herdr 0.8.0), all 18 checks pass including the newok - real herdr: a firstmate capture never moves the pane viewportFault injection: replayed the test's exact scenario (throwaway pane,seq 1 500, read.result.pane.scrollbefore/after) but genuinely moved the viewport between reads via an attached herdr client in tmux copy-mode;offset_from_bottomwent 0 → 29 and the test's exact assertion fired (not ok - a firstmate capture moved the pane viewport), proving the test would fail if a capture scrolled the viewVerified seeded scrollback is real before the captures (max_offset_from_bottom: 481in.result.pane.scroll), so the pass is not vacuousManual doc check: the two new sentences sit directly after the existingpane read --lines N/ 200-line-floor paragraph in 'Current transport behavior', one sentence per line, no duplication of existing content; diff bounded to 2 doc lines + 1 test blockCleanup verified: no leftover fm-lab herdr sessions, no tmux sessions,git status --porcelainclean✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.