Skip to content

feat: glow-terminal-rendering#79

Merged
satyaborg merged 3 commits into
mainfrom
feat/glow-terminal-rendering
Jun 29, 2026
Merged

feat: glow-terminal-rendering#79
satyaborg merged 3 commits into
mainfrom
feat/glow-terminal-rendering

Conversation

@satyaborg

Copy link
Copy Markdown
Owner

Glow-rendered specs and reports in the terminal

Problem

Reading the two artifacts devloop produces pulls you out of the terminal into two other tools. The fzf picker preview shows unstyled raw text: ui_pick_from_file runs --preview 'sed -n "1,80p" {}' (devloop:1069), so selecting a spec shows flat Markdown, which is why a second tool (Obsidian) stays open just to read specs. Reports are worse: the default report format is html (devloop:1451, :1895, :1966), and view_file opens *.html with open/xdg-open (devloop:1711-1716), so finishing a run and reading its report yanks you into Chrome. The moment it hurts: picking a spec from devloop shows raw markdown in the preview pane, and opening its report launches a browser tab, fragmenting one loop across three surfaces.

Outcome

glow renders Markdown in every fzf preview pane (specs, reports, tracks) and in view_file for *.md reports, so reading happens in the terminal. The default report format is markdown, making the default reading path terminal-native and removing the browser from the loop. --report-format html still writes and opens an HTML report via the browser, unchanged. devloop doctor checks for glow. When glow is absent every path falls back to today's behavior, so runs never break.

Acceptance Criteria

AC1: With glow on PATH, the fzf preview command for a .md selection invokes glow with -w "$FZF_PREVIEW_COLUMNS" against the selected file, not sed.
AC2: view_file on a .md argument renders it through glow in a pager when TUI and a TTY are present and glow exists, and falls back to gum pager/$PAGER/cat when glow is absent.
AC3: A run invoked with no --report-format writes .devloop/reports/<slug>.md and writes no .html report.
AC4: A run invoked with --report-format html writes .devloop/reports/<slug>.html, and view_file opens it via open/xdg-open.
AC5: A non-Markdown file passed to the preview or to view_file is never piped through glow (a .html opens in the browser; a .log uses the raw fallback).
AC6: devloop doctor prints a glow status line and returns non-zero when glow is not installed.
AC7: bash scripts/devloop_test.sh passes, including the new glow-present, glow-absent, md-vs-non-md, and markdown-default-report assertions.

Review Trail

Review rounds and the final report are posted as PR comments below.


Spec: .devloop/specs/2026-06-19-glow-tui-rendering.md

Generated by devloop.sh

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 29, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
devloop 9cbdc63 Commit Preview URL

Branch Preview URL
Jun 29 2026, 05:40 AM

@satyaborg

Copy link
Copy Markdown
Owner Author

Devloop Review Round 1

  • Verdict: REJECT

Review 1

Verdict: REJECT

Acceptance matrix

Criterion Status Implementation evidence Test evidence
AC1 FAIL devloop:1120 wraps the fzf placeholder in double quotes: case "{}" in *.md) ... glow -w "$FZF_PREVIEW_COLUMNS" "{}" .... Real fzf replaces {} with the single-quoted string of the line (man fzf, line 739: "replaced to the single-quoted string of the current line"), so at runtime the word becomes case "'/path/file.md'", whose value '/path/file.md' ends in ' and does not match *.md. Control falls to *), and the sed -n "1,80p" "'/path/file.md'" fallback is handed a literally-quoted filename → exit 1, empty output. Glow is never invoked and the .md preview pane renders nothing — a regression from main's working bare-{} sed. Emulating fzf's documented substitution against the exact line confirms branch taken: NON_MD_BRANCH and [exit=1]. bash scripts/devloop_test.sh is green, but the assertion is false-green: the fzf stub at scripts/devloop_test.sh:816 substitutes {} via ${preview//\{\}/$selection} (raw, no single-quoting), so it tests a command string real fzf never runs.
AC2 PASS devloop:1804-1815 (view_file): ui_has_glow (TUI+glow) routes .md through glow "$file" | gum pager, else glow "$file" | "${PAGER:-less}" when TUI+TTY, else bare glow; absent glow falls through to gum pager/$PAGER/cat (:1818-1826). Direct bash, no fzf quoting involved. scripts/devloop_test.sh:836-848 asserts glow <…> + gum <pager> present, and after rm glow asserts glow absent and gum <pager> fallback.
AC3 PASS devloop:2069 defaults report_format="markdown"; devloop:2284-2288 sets REPORT=".devloop/reports/$slug.md" unless html; init_track omits the report-format line for markdown (:3227-3229). scripts/devloop_test.sh:2020-2021 asserts .md written and ! -e .html; :2025 asserts track has no report-format.
AC4 PASS --report-format html parsed at devloop:2088-2097; REPORT=".devloop/reports/$slug.html" (:2285); track records - report-format: html (:3227-3228); view_file .htmlopen/xdg-open (:1797-1800); reports/status list .html (:1787, track_report_path html-first :1888-1893). scripts/devloop_test.sh:2040-2050 (e2e html: .html written, .md absent, track metadata, reports + status include .html); :846-849 (view_file .htmlopen, skips glow).
AC5 PASS view_file discrimination is correct: .htmlopen (:1797), .md→glow (:1803), other→raw (:1818-1826). Preview never pipes non-md through glow (*)sed, :1120). scripts/devloop_test.sh:845 (.html skips glow), :851-852 (non-md raw view, no glow), :843 (non-md preview → sed). Note: in the preview, .md is also never glow'd due to the AC1 bug, so the md/non-md discrimination there is currently vacuous; re-verify after AC1 is fixed.
AC6 PASS scripts/skill_helpers.sh:353 adds devloop_doctor_command glow || status=1; :360-365 prints devloop doctor: ready/not ready and returns $status; missing tool prints [fail] missing command: glow (:198). scripts/devloop_test.sh:1548 ([ok] glow:); :1587-1600 no-glow case asserts non-zero exit, [fail] missing command: glow, not ready.
AC7 PASS n/a bash scripts/devloop_test.sh → exit 0, 56 ok / 0 not ok, 100% project function coverage. The new glow assertions exist, but the two preview assertions (:822-823, :843) are unfaithful to fzf's contract (see AC1) and therefore do not validate the production preview path.

Engineering quality matrix

Area Status Evidence
Correctness FAIL The "{}" double-quoting at devloop:1120 breaks the .md preview in production: glow is never invoked (AC1) and the sed fallback returns nothing, regressing main's working unstyled preview to an empty pane.
Test quality FAIL The fzf stub (scripts/devloop_test.sh:816, :839) does raw {} substitution instead of fzf's single-quoting, so it green-lights a command real fzf never executes. The test meant to guard AC1 cannot catch the regression it exists for.
Maintainability PASS ui_has_glow mirrors ui_has_fzf (devloop:874-876); view_file extends the existing case-on-extension convention; positional-arg threading is consistent.
Architecture boundaries PASS Changes stay in the devloop runtime, skill_helpers.sh doctor, and installers; no feature logic leaks into unrelated modules.
Simplicity PASS Direct Bash, no new abstraction or renderer layer; report-format handled with a single case.
Security N/A No new injection/secret/auth surface; the quoting defect under-executes rather than mis-executes, so no command-injection risk.
Operational safety PASS Every path keeps a graceful fallback (glow absent, gum/PAGER/cat, open/xdg-open); init_track writes atomically via a single { … } > "$file".

Review flags

  • Silent decision: absent - The notable defaults (hidden --report-format, storing report-format in tracks only for html, listing/statusing html reports, reusing the Bash flow over a renderer abstraction) are all recorded in the track's "design tradeoffs"; base main already defaulted reports to markdown and the track acknowledges the pivot to a hidden html opt-in.
  • Scope drift: absent - scripts/install.sh / scripts/install.remote.sh additions are outside the literal AC list but are a necessary corollary of making glow a devloop doctor requirement (AC6) and are listed as changed files in the track.
  • Missing test: present - No faithful fzf-preview assertion exists; the stub diverges from fzf's single-quoting contract, so AC1/AC5-preview are unverified (and currently broken in production).

Findings

  1. [blocker] devloop:1120 - The fzf preview renders an empty pane for every .md selection and never invokes glow. Root cause: real fzf replaces {} with a single-quoted string (man fzf l.739; the gotcha is called out at l.2005-2010), so wrapping it again as case "{}" / glow … "{}" / sed … "{}" double-wraps to "'/path/file.md'"; the case value then ends in ' and misses *.md, and the sed fallback opens a literally-quoted filename (verified: branch NON_MD_BRANCH, sed [exit=1]). This both fails AC1 and regresses main's working preview. Principle: respect the tool's contract — do not re-quote an already-quoted placeholder.
  2. [blocker] scripts/devloop_test.sh:816,839 - The regression intended to lock AC1 is a false-green and let finding feat: port devloop to bun opentui #1 ship. Root cause: the fzf stub uses ${preview//\{\}/$selection}, substituting the raw selection, whereas real fzf single-quotes it; the stub therefore asserts behavior of a command fzf never runs. Principle: a stub that diverges from the real interface's contract provides false confidence rather than coverage.

Missing tests

  • A faithful fzf-preview assertion: the stub must single-quote the selection before {} substitution (as real fzf does) and then assert glow runs for .md and sed runs for non-md. Under the current code this assertion fails (red); it should pass only after finding feat: port devloop to bun opentui #1 is fixed (green).

Fix instructions

  1. In devloop ui_pick_from_file (line 1120), remove the surrounding double quotes from every {} so fzf's own single-quoting applies, matching main's working preview: preview_cmd='case {} in *.md) if command -v glow >/dev/null 2>&1; then glow -w "$FZF_PREVIEW_COLUMNS" {}; else sed -n "1,80p" {} 2>/dev/null; fi ;; *) sed -n "1,80p" {} 2>/dev/null ;; esac'. Keep "$FZF_PREVIEW_COLUMNS" quoted (it is a normal shell variable, not an fzf placeholder).
  2. In scripts/devloop_test.sh, make both fzf stubs (lines 816 and 839) single-quote the selection before substitution, e.g. expanded="${preview//\{\}/\'$selection\'}", so the preview assertions exercise fzf's real quoting. Confirm the suite goes red against the current devloop:1120 and green after fix feat: port devloop to bun opentui #1.
  3. Re-run bash scripts/devloop_test.sh and confirm exit 0 with the corrected, now-meaningful preview assertions.

Notes

  • The bug appears inherited from the spec: the Notes example (spec line 67) also writes case "{}" with double quotes. The implementation must still make glow actually run to satisfy AC1, but the spec example is worth correcting upstream so the next pass does not re-copy it.
  • Verified independently and not in dispute: full suite green (exit 0, 56 ok / 0 not ok, 100% function coverage); devloop doctor returns non-zero on missing glow; the html opt-in writes/lists/statuses .html; and the positional-arg renumbering in init_track (18 args) and synthesize_report (25 args) is correctly aligned at all call sites.
  • After fixing AC1, re-confirm AC5's preview half (md→glow, non-md→sed) with the faithful stub, since that discrimination is currently vacuous.

@satyaborg

Copy link
Copy Markdown
Owner Author

Devloop Review Round 2

  • Verdict: ACCEPT

Review 2

Verdict: ACCEPT

Acceptance matrix

Criterion Status Implementation evidence Test evidence
AC1 PASS devloop:1120 now uses bare {} (no surrounding double quotes): case {} in *.md) if command -v glow …; then glow -w "$FZF_PREVIEW_COLUMNS" {}; else sed -n "1,80p" {} …. With real fzf's single-quoted substitution this becomes case '/path/file.md' in *.md) → matches → glow -w "$FZF_PREVIEW_COLUMNS" '/path/file.md'. Verified by emulating fzf's documented single-quoting: NEW code + .mdglow <-w> <77> </…/preview.md> (pass-1 OLD code → sed … <'/…/preview.md'>, glow never reached). scripts/devloop_test.sh:822-823 ("fzf markdown preview": glow with -w 77 <file> present, sed absent) — the stub now single-quotes the selection (quoted_selection="'$selection'", :809-810), so it exercises fzf's real quoting. Independently confirmed red on pass-1 code, green on this code.
AC2 PASS devloop:1804-1815 (view_file .md): ui_has_glowglow | gum pager, else glow | "${PAGER:-less}" (TUI+TTY), else bare glow; absent glow falls through to gum pager/$PAGER/cat (:1818-1826). Unchanged from pass 1. scripts/devloop_test.sh:836-839 (glow + gum <pager>); :841-843 (after rm glow: no glow, gum-pager fallback).
AC3 PASS devloop:2069 default report_format="markdown"; :2284-2288 REPORT=…/$slug.md unless html; init_track omits report-format for markdown (:3227-3229). scripts/devloop_test.sh:2020-2021 (.md written, ! -e .html); :2025 (track has no report-format).
AC4 PASS --report-format html parsed :2088-2097; REPORT=…/$slug.html (:2285); track records - report-format: html; view_file .htmlopen/xdg-open (:1797-1800); reports/status list .html (:1787, :1888-1893). scripts/devloop_test.sh:2042-2052 (e2e html: .html written, .md absent, track metadata, reports + status include .html); :846-848 (view .htmlopen, skips glow).
AC5 PASS Preview discrimination now genuine (no longer vacuous): .md→glow, non-md→sed (emulation: .logsed </…/note.log>, no glow). view_file .htmlopen, .md→glow, other→raw. scripts/devloop_test.sh:825-826 (non-md preview → sed, no glow); :845, :851-852 (view .html skips glow / non-md raw, no glow).
AC6 PASS scripts/skill_helpers.sh:353 adds devloop_doctor_command glow || status=1; :360-365 prints ready/not-ready and returns $status. scripts/devloop_test.sh:1550 ([ok] glow:); :1589-1602 no-glow case asserts non-zero exit, [fail] missing command: glow, not ready.
AC7 PASS n/a bash scripts/devloop_test.sh → exit 0, 56 ok / 0 not ok, ok - 100% project function coverage. The new glow-present/absent, md-vs-non-md, and markdown-default assertions are present and now faithful to fzf's quoting.

Engineering quality matrix

Area Status Evidence
Correctness PASS The pass-1 preview regression is fixed and verified under real fzf semantics: bare {} lets fzf's own single-quoting drive case matching and pass an unquoted path to glow; emulation shows glow -w 77 <file> for .md and sed for non-md.
Test quality PASS The fzf stubs (scripts/devloop_test.sh:809-811, :817-819) now build quoted_selection="'$selection'" before {} substitution, modeling fzf's contract. Confirmed it is a real guard: red against pass-1 "{}" code, green against this code.
Maintainability PASS ui_has_glow mirrors ui_has_fzf; the inline preview guard is the canonical single-line fzf pattern the spec prescribed; positional-arg threading is consistent.
Architecture boundaries PASS Changes stay in the devloop runtime, skill_helpers.sh doctor, and installers; no feature logic leaks into unrelated modules.
Simplicity PASS Direct inline case guard with sed fallback; no renderer abstraction or wrapper introduced.
Security N/A Path safety is delegated to fzf's own single-quoted {} substitution (canonical, escapes embedded quotes); # shellcheck disable=SC2016 correctly keeps $FZF_PREVIEW_COLUMNS unexpanded for fzf's preview subshell. No new injection/secret/auth surface.
Operational safety PASS Every path retains a graceful fallback (glow absent, gum/$PAGER/cat, open/xdg-open); init_track writes atomically via a single { … } > "$file".

Review flags

  • Silent decision: absent - The defaults (hidden --report-format, storing report-format in tracks only for html, listing/statusing html reports, reusing the Bash flow over a renderer abstraction) are recorded in the track's pass-1 "design tradeoffs"; the pass-2 fixes and their red/green are recorded in pass 2.
  • Scope drift: absent - The only pass-2 changes are the AC1 fix (devloop:1120) and the test-stub faithfulness fix; install.sh/install.remote.sh glow additions (pass 1) are a justified corollary of the doctor requirement (AC6) and are documented in the track.
  • Missing test: absent - The preview path now has faithful .md→glow and non-md→sed assertions, view_file covers md/absent-glow/html/log, doctor covers present/absent, and both default-markdown and html-report e2e flows are asserted. Red/green established for the changed behavior.

Findings

None.

Missing tests

  • None.

Fix instructions

None.

Notes

  • Both pass-1 findings are resolved and independently re-verified under real fzf single-quoting (not merely via the green suite): pass-1 code regresses the .md preview to a bogus quoted sed path with glow never invoked, while this pass yields glow -w 77 <file> for .md and sed for non-md.
  • The spec's own Notes example (spec line 67) still shows the double-quoted case "{}" form that caused finding 1; the implementation correctly diverges from it. Worth correcting upstream so a future pass does not re-copy the buggy snippet.
  • Filenames containing literal single quotes are handled by fzf's own placeholder escaping (matching main's pre-existing behavior); the track's residual-risk note on this is conservative but not a regression and not in scope.

@satyaborg

Copy link
Copy Markdown
Owner Author

Devloop Final Report

Field Value
Final status accepted
Pass count 2 / 5
Final verdict ACCEPT
PR URL #79
Branch feat/glow-terminal-rendering

Acceptance Matrix Summary

Acceptance matrix

Criterion Status Implementation evidence Test evidence
AC1 PASS devloop:1120 now uses bare {} (no surrounding double quotes): case {} in *.md) if command -v glow …; then glow -w "$FZF_PREVIEW_COLUMNS" {}; else sed -n "1,80p" {} …. With real fzf's single-quoted substitution this becomes case '/path/file.md' in *.md) → matches → glow -w "$FZF_PREVIEW_COLUMNS" '/path/file.md'. Verified by emulating fzf's documented single-quoting: NEW code + .mdglow <-w> <77> </…/preview.md> (pass-1 OLD code → sed … <'/…/preview.md'>, glow never reached). scripts/devloop_test.sh:822-823 ("fzf markdown preview": glow with -w 77 <file> present, sed absent) — the stub now single-quotes the selection (quoted_selection="'$selection'", :809-810), so it exercises fzf's real quoting. Independently confirmed red on pass-1 code, green on this code.
AC2 PASS devloop:1804-1815 (view_file .md): ui_has_glowglow | gum pager, else glow | "${PAGER:-less}" (TUI+TTY), else bare glow; absent glow falls through to gum pager/$PAGER/cat (:1818-1826). Unchanged from pass 1. scripts/devloop_test.sh:836-839 (glow + gum <pager>); :841-843 (after rm glow: no glow, gum-pager fallback).
AC3 PASS devloop:2069 default report_format="markdown"; :2284-2288 REPORT=…/$slug.md unless html; init_track omits report-format for markdown (:3227-3229). scripts/devloop_test.sh:2020-2021 (.md written, ! -e .html); :2025 (track has no report-format).
AC4 PASS --report-format html parsed :2088-2097; REPORT=…/$slug.html (:2285); track records - report-format: html; view_file .htmlopen/xdg-open (:1797-1800); reports/status list .html (:1787, :1888-1893). scripts/devloop_test.sh:2042-2052 (e2e html: .html written, .md absent, track metadata, reports + status include .html); :846-848 (view .htmlopen, skips glow).
AC5 PASS Preview discrimination now genuine (no longer vacuous): .md→glow, non-md→sed (emulation: .logsed </…/note.log>, no glow). view_file .htmlopen, .md→glow, other→raw. scripts/devloop_test.sh:825-826 (non-md preview → sed, no glow); :845, :851-852 (view .html skips glow / non-md raw, no glow).
AC6 PASS scripts/skill_helpers.sh:353 adds devloop_doctor_command glow || status=1; :360-365 prints ready/not-ready and returns $status. scripts/devloop_test.sh:1550 ([ok] glow:); :1589-1602 no-glow case asserts non-zero exit, [fail] missing command: glow, not ready.
AC7 PASS n/a bash scripts/devloop_test.sh → exit 0, 56 ok / 0 not ok, ok - 100% project function coverage. The new glow-present/absent, md-vs-non-md, and markdown-default assertions are present and now faithful to fzf's quoting.

Engineering Quality Summary

Engineering quality matrix

Area Status Evidence
Correctness PASS The pass-1 preview regression is fixed and verified under real fzf semantics: bare {} lets fzf's own single-quoting drive case matching and pass an unquoted path to glow; emulation shows glow -w 77 <file> for .md and sed for non-md.
Test quality PASS The fzf stubs (scripts/devloop_test.sh:809-811, :817-819) now build quoted_selection="'$selection'" before {} substitution, modeling fzf's contract. Confirmed it is a real guard: red against pass-1 "{}" code, green against this code.
Maintainability PASS ui_has_glow mirrors ui_has_fzf; the inline preview guard is the canonical single-line fzf pattern the spec prescribed; positional-arg threading is consistent.
Architecture boundaries PASS Changes stay in the devloop runtime, skill_helpers.sh doctor, and installers; no feature logic leaks into unrelated modules.
Simplicity PASS Direct inline case guard with sed fallback; no renderer abstraction or wrapper introduced.
Security N/A Path safety is delegated to fzf's own single-quoted {} substitution (canonical, escapes embedded quotes); # shellcheck disable=SC2016 correctly keeps $FZF_PREVIEW_COLUMNS unexpanded for fzf's preview subshell. No new injection/secret/auth surface.
Operational safety PASS Every path retains a graceful fallback (glow absent, gum/$PAGER/cat, open/xdg-open); init_track writes atomically via a single { … } > "$file".

Implementation Summary

  • Final branch: feat/glow-terminal-rendering
  • Final commit: fad20e5
  • Commit message: fix: glow-terminal-rendering

Commit References

  • pass 1 d567c83 feat: glow-terminal-rendering (README.md, devloop, scripts/devloop_test.sh, scripts/install.remote.sh, scripts/install.sh, scripts/skill_helpers.sh)
  • pass 2 fad20e5 fix: glow-terminal-rendering (devloop, scripts/devloop_test.sh)

Tests Run

  • Verification hook log: not configured
  • Review test evidence: see the acceptance matrix summary above.

Residual Risk

  • No blocking residual risk was recorded by the final review.

@satyaborg satyaborg marked this pull request as ready for review June 29, 2026 05:19
@satyaborg satyaborg merged commit 7d3e78f into main Jun 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant