fix: markdown nesting, worktree removal, CI attribution, and four macOS-only test failures - #207
Open
jonasnobile wants to merge 10 commits into
Open
jonasnobile wants to merge 10 commits into
jonasnobile wants to merge 10 commits into
Conversation
jonasnobile
changed the title
jonasnobile
changed the title
jonasnobile
changed the title
The parser tracked lists in flat state (`in_list`, `list_ordered`, `list_items`), which can only describe one list at a time. A nested list therefore clobbered the list around it: `Start(List)` cleared the outer list's items and overwrote its ordered-ness, and the inner `End(List)` closed the outer list too. The numbering disappeared, every item after the nesting point fell out of the list and rendered as a bare paragraph, and the leftover `End(Item)` events appended empty bullets at the end. Lists and items now live on a stack, and a list item holds blocks rather than a single inline run, so an item can carry several paragraphs, a nested list or a fenced block. Follow-on fixes that fall out of it: ordered lists honour their first number (a list written `3.` no longer restarts at 1), a code block indented under an item stays in that item with syntax highlighting and its own chrome instead of being hoisted after the list, and a nested table renders in place. Code-line and table-row layout moved into `code_line_units` and `table_units` so the nested and top-level paths share the selection offset accounting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
A quote collected only its inline content, which is the same modelling mistake list items had. Every quoted paragraph merged onto one line, and a list inside a quote was emitted after it as a separate top-level node, because the quote was not on the block stack that routes finished blocks. `Node::Blockquote` now holds blocks and a quote is a `Frame` like a list item, so quotes and lists nest in each other either way round. Quoted text reads dimmer and italic, and a paragraph sets its own colour, so an ancestor cannot paint over it. That style now travels down as a `BlockStyle` the container hands to the blocks inside it, which also drops the hardcoded body colour from the inline renderer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
…en it does not Closing a worktree reported "failed to remove directory '<path>'" and nothing else. `GitError::RemoveFailed` kept the io cause in `#[source]`, which neither the toast nor the log walks, so the one sentence the user gets repeated the path it had already named and dropped the reason. The cause is in the message now. `remove_dir_all` walks the tree and then removes the directory itself, so anything writing into the checkout during that walk (a watcher that outlived the shell it came from, Spotlight, Finder) leaves the final rmdir reporting a directory that is not empty. That is a race, not a verdict: it now gets a few more attempts, and only for the error kinds that can be transient. A permission error still fails on the first try, since retrying it only delays the same report. A quarantine that can be neither deleted nor put back used to stay on disk as a whole checkout under a hidden name, invisible to the user and swept up by nothing. The next removal in that directory reclaims it, skipping any quarantine a concurrent removal is using. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
`successful_hook_pty_exit_removes_real_worktree` failed on macOS and `hook_exit_after_stash_keeps_changes_written_since_the_stash` failed whenever the machine was busy. Both created the hook PTY through `ShellType::for_command`, which is the production path: `$SHELL -ic`, an interactive shell, so a hook sees the user's aliases and environment. In a test that only buys a dependency on what the developer's profile costs to start, and an interactive zsh that takes 1.6s to reach the command spends most of a 2 second budget before the hook has run. What these tests cover is PTY exit handling and worktree removal, not shell resolution, so the command runs in a bare shell. The wait is a named constant and generous: it is there to turn a hang into a failure, not to assert how fast a PTY starts. The module now runs in 0.24s rather than 2.4s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
…up first The two daemon-backed tests in this file never passed on macOS. They watched for `remote.json` under `$XDG_CONFIG_HOME`, but `config_root()` resolves through `dirs::config_dir()`, which is `~/Library/Application Support` there. The test redirects both that and `HOME` into its isolated root, so the daemon was contained either way; it published a second into the run, to the path nobody was watching, and each test then waited out its full 60 second timeout. They look for whichever path the platform uses now. With the daemon found, a second race showed up under load: `remote.json` says only that the port is published, and the first CLI call carries a one-off token registration on top of its command. The CLI gives a request 5 seconds, which a busy machine can spend on that round trip alone. Startup now ends when a CLI call has actually been answered, so the wait sits where it proves nothing rather than inside an assertion. The target runs in 4s instead of 120s, and holds up with a full `cargo test --workspace` running beside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
Two legacy worktree recovery tests failed on macOS. `GitFixture` builds its root from `std::env::temp_dir()`, which there is handed out behind a symlink (`/var/folders/...` is `/private/var/folders/...`). Git's worktree registry reports resolved paths, and `registered_checkout_root` matches a row against them lexically, on purpose: it must not touch the filesystem, or a checkout deleted from disk would stop being sweepable. So the prefix never matched, the recovery under test never ran, and the row was dropped. The fixture starts from the resolved path, which is what a checkout anywhere outside the temp directory already is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
A worktree showed a failed CI run it had nothing to do with: a deploy/staging run belonging to the repo's default branch. Two causes, one behind the other. Okena creates a worktree branch with `git worktree add -b <branch> <path> origin/<default>`, and git records a remote-tracking start point as the new branch's upstream. So a branch that has never been pushed comes out tracking `main`. Creation now passes `--no-track`; `git push -u` records the real upstream once there is something to record. Everything keyed on "the branch's pushed commit" then read that upstream without checking whose it was, so for such a branch it resolved to main's tip. The branch CI lookup asked GitHub for the check runs of that commit and got main's, and PR resolution compared a PR head against it. The CI path now takes the upstream only when it is this branch's own counterpart, treating the default branch as a base rather than a counterpart. A branch deliberately tracking a differently named remote branch keeps its own answer. Reported from a checkout whose upstream was `origin/main` at 74aa8f43c while the branch itself was at 815973353. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
Closing a worktree kept failing on a checkout that nothing was wrong with. The project's own containers bind-mount it: a Postgres data directory, an object store, a dev server's state. Deleting it while they run is a race that cannot be won, because they keep writing into the tree while the delete walks it, and the final rmdir then reports a directory that is not empty. Retrying does not help against a live database. Closing already unloaded the project's services from the ServiceManager, but that only stops Okena from supervising them; nothing ever told Docker anything. A grep for `"down"` across okena-services, okena-daemon-core and okena-workspace found nothing: compose was only ever driven per service, with `up`, `stop` and `restart`. The stack now comes down before the checkout is deleted. That also settles the other half of the same bug: a removal that did succeed left the compose project running against files that no longer existed, which is how a stack outlived its worktree by hours. Best effort by design. A stack that will not come down leaves the removal to fail closed with the io cause, now with the compose failure alongside it, rather than adding a new way to block a close. Docker not running at all means nothing holds the checkout either, and that close should still go through. Diagnosed from a checkout whose delete kept failing while `docker compose ls` showed its stack running(7), with four containers mounting the directory and the Docker VM holding descriptors inside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
…eady removed A worktree close reported `Permission denied` against the checkout root. What actually refused was one directory inside it: a `node_modules` that Docker Desktop holds as a mount point for a named volume and protects with a `deny delete` ACL. `std::fs::remove_dir_all` reports the io error without the path it happened on, so the report sent the reader to the wrong directory, and a failure that is completely explainable looked like the checkout itself being unreadable. Removal now walks the tree itself and annotates the failure with the entry that refused, keeping the error kind so the retry decision still reads the real one. The second half is worse and was invisible. A delete removes as it walks, so a refusal part-way through leaves the checkout short of whatever went before it. The failure path renames the directory back and reports that the checkout "remains open", which reads as nothing having happened: in the reported case fifteen tracked files were already gone. The message now says how many, and that `git restore .` brings back the tracked ones while untracked files it removed are gone for good. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
`check-windows` could not compile them: one stages its refusal with unix permissions, the other creates a symlink, and neither `os::unix` exists there. The refusal test is now unix-only, following the symlink-alias test beside it; the tree-removal test still runs everywhere, with only the symlink part gated, so Windows keeps the coverage that a clear tree is removed whole. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut
jonasnobile
force-pushed
the
fix/markdown-nested-lists
branch
from
September 21, 2026 15:04
0782c02 to
ea0285b
Compare
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
1 participant
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.
Seven commits, four unrelated areas that came up in one session. Split by
concern so they read on their own.
Markdown: nested lists and quotes (2 commits)
A document with a nested list rendered wrong in the file viewer's preview: the
outer list lost its numbers, every item after the nesting point turned into a
bare paragraph, and a run of empty bullets appeared at the end.
List state was flat (
in_list,list_ordered,list_items), so it could onlydescribe one list at a time. A nested list cleared the outer list's items,
overwrote its ordered-ness, and closed it early. Blockquotes had the same
modelling mistake: both an item and a quote were a single
Vec<Inline>, soquoted paragraphs merged onto one line and a list inside a quote was emitted
after it.
Items and quotes now hold blocks, and a
Framestack routes each finished blockto the innermost open container. Ordered lists honour their first number, a code
block indented under an item stays in that item with highlighting and chrome,
and a nested table renders in place. Quoted text keeps its dimmed italic look
through a
BlockStylethe container hands down, since a paragraph sets its owncolour and an ancestor cannot paint over it.
Worktree removal (1 commit)
Closing a worktree reported
failed to remove directory '<path>'and nothingelse: the io cause sat in
#[source], which neither the toast nor the logwalks. The cause is in the message now.
Two behaviour fixes behind it. A delete that loses a race with something still
writing into the checkout gets a few more attempts, and only for error kinds
that can be transient (a permission error still fails on the first try). And a
quarantine that can be neither deleted nor restored no longer sits on disk as a
whole checkout under a hidden name: the next removal in that directory reclaims
it, skipping any a concurrent removal is using.
CI attributed to the wrong branch (1 commit)
A worktree showed a failed
deploy/stagingrun belonging to the repo's defaultbranch. Two causes, one behind the other.
git worktree add -b <branch> <path> origin/<default>makes git record thestart point as the new branch's upstream, so a branch that has never been pushed
comes out tracking
main. Creation passes--no-tracknow.Everything keyed on "the branch's pushed commit" then read that upstream without
checking whose it was, so it resolved to main's tip: the branch CI lookup asked
GitHub for the check runs of that commit and got main's, and PR resolution
compared a PR head against it. The CI path takes the upstream only when it is
this branch's own counterpart; a branch deliberately tracking a differently
named remote branch keeps its own answer.
Four tests that could not pass on macOS (3 commits)
Found while verifying the above. All pre-existing, each confirmed against a
clean tree before being touched.
okena-daemon-core$SHELL -ic, so the test spent an interactive zsh startup (1.6s on the reporting machine) out of a 2 second budget. They now use a bare shell; the module runs in 0.24s instead of 2.4s.cli_over_the_remote_apiremote.jsonunder$XDG_CONFIG_HOME, but macOS publishes under~/Library/Application Support, so both waited out a 60 second timeout on every run. Startup now also ends only once a CLI call has been answered, which closes a second race under load. 4s instead of 120s.okena-workspacecargo test --workspace --no-fail-fastis clean, and the CLI target holds upwith a full workspace run beside it.
Noted, not changed
registered_checkout_rootmatches paths lexically, so a worktree under asymlinked path is not recovered. The lexical match is deliberate (a deleted
checkout has to stay sweepable), so fixing it needs a decision about missing
paths rather than a quiet change here.
🤖 Generated with Claude Code
https://claude.ai/code/session_01RkbvFzpmyCRY4DBp7oJHut