Skip to content

docs(demo): keep demos off the real login, and wait on the right text - #71

Merged
alukach merged 1 commit into
mainfrom
fix/demo-recorder
Nov 6, 2026
Merged

alukach merged 1 commit into
mainfrom
fix/demo-recorder

Conversation

@alukach

@alukach alukach commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Fixes from the code review of demo/ (#59, #60, #69). The most important: a demo could read or remove your real production login.

What changes

  • Demos get an environment of their own. The CLI keys its keyring entries by environment (cache::env), and replacing HOME doesn't move the keyring. The recorder left SOURCE_PROXY_URL at production, so a demo of auth status would have shown your real account in a public SVG, and auth logout would have deleted your real session. SOURCE_PROXY_URL now points at the stub, so each run is its own environment (127.0.0.1:PORT). One gap remains, noted in a ponytail: comment and in demo/README.md: a demo that saves a session or credentials still writes an entry for that environment to the OS keyring. Closing it would need a way to turn the keyring off in the CLI, so the docs say to leave sign-in out of demos.
  • see() counts only what its match covers as seen. It used to count everything received so far, so a prompt that arrived with the matched text was gone before the next prompt() waited for it.
  • run() skips the shell's echo of the command, so s.run("… bob"); s.see("bob") waits for the command's output, not the typing.
  • SOURCE_PAGER=cat, so a list taller than 34 rows doesn't open less and stall the recording.
  • The SVG rewrite looks for leftover CSS only inside tags. Before, any demo whose output contained "style" (e.g. "lifestyle") stopped with "svg-term wrote CSS this can't rewrite".
  • The CLI talks to 127.0.0.1, where the stub listens, not localhost, which may try IPv6 first. The URLs in demos now read http://127.0.0.1:PORT.

Not in this PR

  • Stub fixes (error shape, invitation state, a GET that changed memberships): in a separate PR.
  • Cleaning up a failed demo's shell: the review reported that a failed demo leaves bash and the CLI running. That didn't reproduce: the failure ends the recorder, which closes the terminal, and that hangs up both. Checked with main's record.py as well.
  • A setting to turn the keyring off, for the gap above.

Testing

  • A demo printing a lifestyle with class="g" is done, waiting for done (also in the typed command), then prompt(), then a product list without --no-pager. It records on this branch. On main it fails: see("done") matches the echo and the prompt waits fall out of step. On the raw svg-term output of that recording, main's without_css() raises and this branch's keeps the text.
  • A demo that fails on purpose inside product list -i leaves no shell or CLI running, on this branch or main.
  • The two demos from chore(demo): attach demo SVGs to PRs with gh instead of hosting them #69 re-record cleanly. The CLI prints http://127.0.0.1:PORT URLs.
  • Not run: any auth command in a demo, which would touch the real keychain (AGENTS.md: never in manual runs). The isolation comes from reading cache.rs: the keyring account is account(env, name), and the environment comes from SOURCE_PROXY_URL.
  • cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test: 165 pass. No Rust changes.

🤖 Generated with Claude Code

- Give each run an environment of its own (SOURCE_PROXY_URL at the stub),
  since the cache keys keyring entries by environment and HOME doesn't
  move the keyring: a demo could read or remove the real login.
- see() counts only what its match covers as seen, so a prompt that
  arrives with the match is still there for the next wait.
- run() skips the shell's echo, so waiting for text that's also in the
  command waits for the command's output.
- SOURCE_PAGER=cat, so a list taller than the screen doesn't wait for a key.
- The SVG rewrite looks for leftover CSS only inside tags, so a demo that
  prints "style" no longer fails.
- Point the CLI at 127.0.0.1, where the stub listens, not localhost.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Claude finished @alukach's task in 17s —— View job


✅ No blocking issues — safe to merge.

I read the diff only and did not run anything. Checked:

  • see() / after_visible: The visible-character offset is counted the same way ANSI.sub strips. A match that ends at visible position 0 leaves seen unchanged. Escape sequences after the matched text are left unconsumed, which is intended.
  • run() echo skip: seen sits before the typed command, so the first \n is the Enter echo. The wrapping limitation is already noted in a ponytail: comment.
  • without_css: <[^>]*> is safe as long as svg-term escapes > in text and attribute values. That holds for 2.1.1 as far as I can tell. The tags lambda reads s when it is called, so it sees the current value.
  • Isolation: Setting SOURCE_PROXY_URL to the stub gives each run its own keyring and cache environment, and the remaining keyring-entry gap is documented. Demos of creds, or anything else that uses the proxy URL, will now hit the stub. They get its explicit 404 message, which is the intended failure mode.

Simplify (ponytail)

  • demo/record.py, without_css leftover check: re.findall(r"<style|\sstyle=", "".join(re.findall(r"<[^>]*>", s))) re-scans every tag. You could keep the tag text from the first pass, or just test re.search(r"<style|<[^>]*\sstyle=", s).
  • demo/record.py module docstring: the edited sentence runs past the other lines' wrap width. Re-wrap it.
    · branch fix/demo-recorder

💰 Estimated review cost: $0.13 · 0m17s · 4 turns

@alukach alukach changed the title fix(demo): keep demos off the real login, and wait on the right text docs(demo): keep demos off the real login, and wait on the right text Nov 6, 2026
@alukach
alukach merged commit e082efb into main Nov 6, 2026
10 checks passed
@alukach
alukach deleted the fix/demo-recorder branch November 6, 2026 22:30
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