Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #16 +/- ##
=======================================
Coverage 82.90% 82.90%
=======================================
Files 17 17
Lines 983 983
Branches 110 110
=======================================
Hits 815 815
Misses 122 122
Partials 46 46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…yling lost in gp-sphinx migration
Commit ``c822f02`` deleted ``docs/_static/css/custom.css`` when adopting
gp-sphinx, which dropped 618 lines of CSS — including the project-specific
admonition classes that make ``docs/recipes.md`` and
``docs/topics/prompting.md`` readable:
* ``.admonition.prompt`` — speech-bubble prompt block (💬 icon,
accent border, italicized body)
* ``.admonition.agent-thought`` — muted gray-bar "narration" aesthetic
* ``span.prompt`` — inline italic with curly quotes
* ``div.system-prompt`` /
``div.server-prompt`` — labeled dark code panels
The behavior piece (``prompt-copy.js`` + ``copybutton_selector``) survived
the migration, so copy-on-prompt still worked; only the *visual cue* that
made these blocks look distinct was gone. Every ``:class: prompt``
admonition and every ``[...]{.prompt}`` inline span was rendering as a
plain admonition / plain italics.
Restored the rules verbatim from ``git show c822f02^:docs/_static/css/custom.css``
into a new ``docs/_static/css/project-admonitions.css`` — named to signal
intent (these are libtmux-mcp-specific classes, not theme overrides) — and
registered via ``app.add_css_file()`` in ``conf.py::setup`` alongside the
existing ``app.add_js_file("js/prompt-copy.js")`` registration so order is
deterministic (project CSS loads after ``_gp_setup(app)`` runs).
gp-sphinx's ``merge_sphinx_config`` does not set ``html_css_files`` and its
``setup()`` only adds ``js/spa-nav.js``, so there is no collision risk
with upstream theme CSS. ``DEFAULT_HTML_STATIC_PATH = ["_static"]`` already
covers serving files from ``docs/_static/``.
…-sphinx SPA swap
``copybutton_selector`` in ``docs/conf.py`` is
``"div.highlight pre, div.admonition.prompt > p:last-child"`` — we copy
*prompt text* as well as code. On full-page load,
``sphinx-copybutton`` iterates that selector and inserts a ``.copybtn``
after every match, then binds ``new ClipboardJS('.copybtn', ...)``;
ClipboardJS uses delegated listening on ``document.body``, so those
clicks keep working across SPA DOM swaps.
On SPA navigation, however, gp-sphinx's
``spa-nav.js::addCopyButtons`` iterates ``"div.highlight pre"`` only —
it does NOT re-attach buttons to ``.admonition.prompt > p:last-child``.
After an SPA swap, prompt-heavy pages like ``/recipes/`` (no code
blocks, eight ``.admonition.prompt``) render naked: no copy
affordance at all. This is the user-visible regression introduced by
the gp-sphinx migration (``c822f02``).
This shim fills the gap:
* Capture the first live ``.copybtn`` that appears anywhere in the
document as a reusable template, so ``sphinx-copybutton``'s
locale-specific tooltip and tabler-copy SVG are preserved exactly.
``FALLBACK_COPYBTN_HTML`` covers the rare case where the user's
first page has no ``.copybtn`` anywhere (landing with no code and
no prompts) before any SPA nav.
* On every SPA swap, re-insert ``.copybtn`` siblings after any
``.admonition.prompt > p:last-child`` lacking one, assigning the
``<p>`` a ``mcp-promptcell-N`` id and wiring the button's
``data-clipboard-target``. The inserted element plugs into
ClipboardJS's existing body delegation *and* the project's
capture-phase ``prompt-copy.js`` delegation transparently — no
extra click handlers required.
Initial-load ordering: at deferred-script execution time
``document.readyState`` is ``"interactive"``, not ``"loading"``, so
a naive readyState check would run our initial pass eagerly —
*before* sphinx-copybutton's DOMContentLoaded handler fires, letting
our fallback template beat sphinx-copybutton's proper one. We gate
on ``"complete"`` instead, so we always register a DOMContentLoaded
listener (which fires after sphinx-copybutton's, registered earlier
when ``readyState`` was ``"loading"``) unless the document is already
fully loaded.
Verified end-to-end in real Chromium via Playwright:
* Fresh load of ``/recipes/``: 8 buttons, all with
sphinx-copybutton's ``#codecell{N}`` targets and tabler-copy SVG
— shim is inert.
* SPA-nav away then back to ``/recipes/``: 8 buttons re-inserted
with ``#mcp-promptcell-{N}`` targets.
* Click on a shim-inserted button: ``prompt-copy.js``'s
capture-phase delegation fires, markdown-preserving
``navigator.clipboard.writeText`` succeeds, ``.success`` class +
``"Copied!"`` tooltip + tabler-check SVG swap — identical
feedback to a fresh-load click.
* ``/topics/prompting/`` (code-only, no prompts): 4 buttons
re-added by gp-sphinx, clicks still fire through ClipboardJS
delegation.
* Multiple SPA navs in a row: idempotent, no double-insertion.
The correct upstream fix is in gp-sphinx — its ``addCopyButtons``
should iterate the full ``copybutton_selector`` (or dispatch a
``spa-nav-complete`` event that consumers like ``sphinx-copybutton``
can hook). Until then, this project-local shim keeps the docs
behaving.
52bb512 to
581e80e
Compare
tony
added a commit
that referenced
this pull request
Apr 17, 2026
why: AGENTS.md §255-287 requires doctests on pure helpers. Most of
the pure helpers introduced by PR #15 have them
(``_validate_channel_name``, ``_validate_logical_name``,
``_validate_buffer_name``, ``_truncate_lines_tail``,
``_tmux_argv``, ``_pane_id_sort_key``). The critic pass surfaced
``_allocate_buffer_name`` as the lone remaining pure helper
without a doctest — an oversight when commit 272396d introduced
the module.
Loom review Suggestion #16 (finalized).
what:
- Expand the docstring on
src/libtmux_mcp/tools/buffer_tools.py::_allocate_buffer_name with
a NumPy-style ``Examples`` block covering:
* prefix contract (``"libtmux_mcp_"``),
* logical-name suffix preservation,
* 32-hex-character uuid nonce length,
* empty-string and ``None`` both collapsing to the ``"buf"``
fallback.
- The docstring also expands the rationale for why the name has the
exact shape it does (privacy prefix + collision-resistant nonce
+ logical suffix), so future maintainers don't reduce the
structure and accidentally re-introduce either the OS-clipboard
read risk or the cross-agent collision risk that commit 272396d
fixed.
9 tasks
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.
Summary
Fixes two regressions introduced by the gp-sphinx theme migration (c822f02):
c822f02deleteddocs/_static/css/custom.css(618 lines). Every:class: prompt/:class: agent-thought/:class: server-promptblock and every inline[...]{.prompt}span was rendering as plain admonition / plain italics.addCopyButtons()only iteratesdiv.highlight pre—.admonition.prompt > p:last-childbuttons never reappear after a swap. Prompt-heavy pages like/recipes/(zero<pre>, eight.admonition.prompt) render without any copy affordance until a hard refresh.Commits
6d0c084— Restore project-specific admonition CSS in a newdocs/_static/css/project-admonitions.css(lifted verbatim fromgit show c822f02^:docs/_static/css/custom.csslines 195–360). Registered viaapp.add_css_fileinconf.py::setupalongside the existingprompt-copy.jsregistration.52bb512— Newdocs/_static/js/spa-copybutton-reinit.jsshim: capture sphinx-copybutton's real.copybtnas a template on DOMContentLoaded, then on every SPA swap re-insert buttons on prompt-admonition<p>elements that lack one. Inserted buttons plug into ClipboardJS's body-delegated listener and the project's capture-phaseprompt-copy.jsdelegation transparently.Verification
Built locally with
just build-docs; exercised the SPA flow in real Chromium via Playwright:/recipes/: 8 buttons with sphinx-copybutton's#codecell{N}targets and tabler-copy SVG — shim is inert./recipes/: 8 buttons re-inserted with#mcp-promptcell-{N}targets..successclass +"Copied!"tooltip + tabler-check SVG swap — identical feedback to a fresh-load click./topics/prompting/(code-only, no prompts): 4 buttons re-added by gp-sphinx, clicks still fire through ClipboardJS delegation.Follow-up (out of scope)
The right long-term fix belongs in gp-sphinx: have
sphinx_gp_theme/theme/static/js/spa-nav.jseither dispatch aspa-nav-completeCustomEventor iterate the fullcopybutton_selector. That's a PR against the shared theme repo; this shim is the project-local workaround.Test plan
just build-docssucceeds./recipes/, click a prompt copy button — confirm clipboard has the text and tooltip flips toCopied!./and back to/recipes/— confirm copy buttons still appear and still copy./topics/prompting/— confirm code-block copy buttons still work.