Skip to content

Toad suite: find the conftest.py:93 hang, fix link and Core-type drift - #1375

Closed
trissim wants to merge 20 commits into
mainfrom
hotfix/toad-suite-main
Closed

trissim wants to merge 20 commits into
mainfrom
hotfix/toad-suite-main

Conversation

@trissim

@trissim trissim commented Oct 30, 2026

Copy link
Copy Markdown

Toad main's required suite check: why it hangs, why it's red, and the first batch of fixes at their owners. Built on #1368's ok head (e65569e), which already fixes the unread/read-ledger drift.

What conftest.py:93 waits on (the "2-minute hang")

OwnProcessTest.runtest (tests/conftest.py:93) waits in BoundedRun.run, which returns only when the child's stdout/stderr pipes close. In run 41103293550, gw0 ran test_hidden_session_retirement (both tests) and each ran out the full --test-timeout of 165 s (dumps at 18:26:09, 18:28:54, 18:31:40). Reproduced locally: the child reports 1 failed in 13.70s, and the parent reports in 166.62s. The app's close dies on NoMatches: SessionThreadSidebar (src/toad/screens/main.py:459, from main.py:439 during workspace_chrome.refresh_layout). Its helpers (the observer and others) keep the inherited pipes open, and stop_test_children runs only in the parent's finally, after the kill. Not fixed in this commit yet: the mailbox MainScreen is mounted and composed (['Center']), and the trace of why its sidebar query fails continues.

Wall time (separate structural defect)

CI ran 2 xdist workers over 715 items and finished 468 in 28.7 min (16.3 tests/min), so a full pass needs about 44 min against timeout-minutes: 30. Even with zero failures, the suite can't finish. Not addressed here.

Fixed in this commit

  • src/toad/markdown_links.py:213: gfm-like linkify fuzzy-linked file.md → http://file.md (.md and .py are TLDs). Linkify now owns written schemes and e-mail only; schemeless spellings go to TextTokenRule. CodeInlineTokenRule again links only a whole-span reference; 1325acc reduced it to pass.
  • tests/textual/test_{arrival_fade,app_close_order,checkpoint_first_source,snapshot_cancellation}.py: four hand-built CoordinationChangedUpdate copies passed worktree=str(...) ('str' has no to_wire). They are deleted in favor of support.coordination.registered_coordination_fact.
  • tests/textual/test_file_links_in_agent_text.py, tests/app/test_markdown_file_links.py: these asserted our own toad-file: href spelling, and hrefs are now LinkHref JSON. They now assert the typed LineLink.click. UI.hyperlink_at is deleted (no other callers). The bare-https assertion pinned pre-4e493ec3f behaviour ("linkify is syntax") and now names the exact ExternalProjectLink.
  • test_acp_log_link.py:66 now uses the src producer AcpLogLink.markdown(path).
  • test_channel_scope_rows.py:18, test_command_pane_and_resume_modal.py:61: Checkout drift.
  • test_cursor_request_results.py:33: AcpSessionId / RootId drift.

Targeted results (one process per test, timeout -s INT 300 -p no:xdist -o faulthandler_timeout=60)

  • P1022: migrate stale Core test callers and verify main pushes #1368 head, the 61 CI-failed tests: 8 pass / 53 fail.
  • This branch, first batch of the 17 touched tests: 7 pass / 10 fail. That was before the RootId, inline-code and linkify fixes, so a rerun is pending.
  • The ui.until cluster (unread ×3, channel_scrolling, any_mode and others) is load-sensitive: test_direct_message_unread_clears_on_opening passes alone (33.6 s) and failed at load 23.

Still open

hidden_session_retirement (NoMatches + the 165 s hang), test_dm_peer_replacement:34, test_divider_only_viewport_stays_unread:30, arrival_fade:87, channel_tab_sharing, prompt_routing '!', tab_history, channel_mentions, sessions_share_one_stdio_process, conversation_value remove_originals, channel_read_ahead .service, archive Retain, acp_log_view, error_notifier, editor_keys, compaction_turn_events, application_actions, and the 30-minute wall time.

Assumed: bare example.com / www.example.com in agent text becomes a project file-search link (the pre-10-15 behaviour), not a website.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu

trissim and others added 17 commits October 30, 2026 10:06
Carry NoSessionObservation and Checkout call migrations from #1364; delete both seen_sequences test forwarders and inspect conversation cursors directly. Restore main-push verification and its push-specific base/head selection.
Carry NoSessionObservation and Checkout call migrations from #1364; delete both seen_sequences test forwarders and inspect conversation cursors directly. Restore main-push verification and its push-specific base/head selection.
- markdown_links: linkify owns written schemes and e-mail only (fuzzy_link off), so
  schemeless spellings such as file.md and declarations.py resolve as project paths
  again instead of http://file.md; inline code links only when the whole span is a
  reference (regressed in 1325acc).
- Tests build coordination facts through support.coordination's
  registered_coordination_fact instead of four hand-built copies passing a str worktree.
- Link tests assert typed LineLink actions (link.click) instead of our own href
  spelling; the hyperlink_at accessor that pinned that spelling is deleted.
- Checkout, AcpSessionId and RootId typed where Core now requires them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
Copilot AI balanced review requested due to automatic review settings October 30, 2026 20:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim enabled auto-merge October 30, 2026 20:37
trissim and others added 3 commits October 30, 2026 17:05
…on mount

The "2-minute hang at conftest.py:93" was BoundedRun.run reading the child test's pipes to
EOF while helpers its app left running (after a failed close) still held them, so a test
that failed in 14 s was killed at --test-timeout 165 s. The child now stops what its test
started in pytest_sessionfinish; the runner's finally stays for a killed child.
Measured on the unfixed test: 166.62 s -> 20.68 s.

test_hidden_session_retirement passed Path(owner.worktree) to MainScreen.from_thread,
which takes a Checkout: on_mount's update_title raised on .path, Toad's _handle_exception
swallowed it, Textual's _pre_process ended the screen's loop and pruned its children, and
sidebars() then raised NoMatches. Pass the Checkout.

Registration.remove_originals is gone: retire threads through comms.threads.delete, as
test_conversation_kind already does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
…istory surface

test_dm_peer_replacement waited on read_worker, which leaving the tab legitimately cancels,
and the paused mark keeps running in its thread after that cancel. It now waits on the mark
itself, so its assertions run after the mark has really happened.

test_channel_read_ahead used names deleted on main: CoordinationAccess.service (now the
observation's service), WireHistory.messages (fragments since 12b1f40), history.source
(page_source.loader) and channel_display_page's before/limit keywords (PageBounds now). Its
read-ahead threshold is unchanged and still fails: only the tail and one page are read.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
…mode

TabHistoryButton.sync (src/toad/widgets/side_bar.py:283) shows availability as
disabled; the -unavailable class the test probe read is set by nothing, so a fresh
app always looked navigable. test_prompt_routing still sent "!literal" as text and
pinned "!" as text, which cfe1bb4 deliberately made the shell-mode switch on an
empty prompt; tests/textual/test_prompt_shell_mode.py owns that behavior.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
trissim added a commit that referenced this pull request Oct 31, 2026
The suite needs about 44 minutes on 2 workers (#1375) and was cut off at
30, around 70%, on every branch. No run has ever reported the last 30%.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trissim added a commit that referenced this pull request Nov 1, 2026
markdown-it's gfm-like linkify had fuzzy_link on, so file.md and
declarations.py rendered as links to http://file.md and
http://declarations.py. Written schemes and e-mail addresses stay with
linkify; schemeless spellings are left to TextTokenRule's project-path
resolution. A code span links only when the whole span is a reference,
so `run file.md` stays a command.

Salvaged from #1375 (its markdown_links.py hunk only; the suite
migrations there are obsolete since #1397). Reproduced with
markdown-it-py: before, both paths became http links; after, they stay
text while https:// and mailto: still link.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016gZLdttfQjtizEYZGek9Gh

trissim commented Nov 1, 2026

Copy link
Copy Markdown
Author

Salvaged the one still-needed part of this PR onto main: the markdown_links.py fix, which stops linkify turning file.md into an http link and links a code span only when the whole span is a path. Reproduced before and after with markdown-it-py. Closing the rest as obsolete: the suite migrations and the conftest hang hunt stopped mattering once the full suite became release-qualification only (#1397, c5ba1fd). Stale-backlog pass at Tristan's request.


Generated by Claude Code

@trissim trissim closed this Nov 1, 2026
auto-merge was automatically disabled November 1, 2026 21:35

Pull request was closed

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.

2 participants