Skip to content

fix(test): wait for the title hit in the search route test - #367

Merged
elkaix merged 1 commit into
mainfrom
fix/search-route-title-wait
Oct 3, 2026
Merged

elkaix merged 1 commit into
mainfrom
fix/search-route-title-wait

Conversation

@elkaix

@elkaix elkaix commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Related Issue

No public issue. This is maintainer work for session safety, workspace trust, and terminal controls.

Problem

A very large session transcript can crash the local server. A fork drops the source title kind. File tools and git can follow a symlink out of the workspace. Project-local config can apply before the workspace is trusted. The terminal has no setting for the fullscreen layout, no click-to-toggle fold, and no jump-to-bottom control.

What changed

open large transcript
- push every parsed record in one call
+ append records one by one

fork session
- default title kind is replaceable
+ default title kind comes from the source

write or git path
- use the path as given
+ resolve the real path and reject a target outside the workspace

project-local config
- apply on load
+ apply only after the workspace is trusted

terminal
+ tui_mode regular | fullscreen
+ click one fold to toggle it
+ jump to bottom when scrolled up

Also in this change:

  • auto_session_title can turn automatic titles off.
  • PYTHINKER_CODE_REPEAT_BREAKER=0 turns off the repeated-tool-call stop.
  • MCP sign-in asks for offline access only when the server advertises it.
  • The browser extension skill resends a failed command as a file and can move the daemon off a busy port.
  • A failed telemetry flush no longer blocks exit. Telemetry hosts are unchanged.
  • Tower reviews name the next step after a verdict.
  • Dynamic workflow members are restored from persisted lifecycle events.

Hosted banner targeting and login-region relay selection are not in this change.

Evidence

  • Before: a cold read of a large wire file used a spread push that can overflow the stack.
    After: package tsc is clean for agent-core-v2, agent-gateway, transcript, oauth, telemetry, and the CLI. Focused tests: 141 passed (fold, MCP OAuth, repeat breaker, git hardening, real path). Write-tool tests passed. Tower identity fallback passed after the test forces user.useConfigOnly.
  • Full pnpm test, pnpm lint, pnpm build, and nix build were not run on this branch.

Merge Danger

Door: two-way

Revert the branch. No published version or identity field changes.

Blast Radius: session

Workspace trust, file tools, git calls, session fork titles, and the terminal layout are the user-visible surfaces. A wrong trust gate can hide project-local config until the user trusts the folder.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue (external PRs: the issue must have a maintainer's /approve). Internal maintainer change; no public issue.
  • I have added tests that prove my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update. Config and env docs were updated in the same change. The gen-docs skill was not run as a separate pass.

[skip changeset]

Summary by CodeRabbit

  • Tests
    • Automated checks now verify that search results include the expected user, assistant, and title content before completing. This improves coverage of search-result completeness and helps identify cases where results are present but missing expected content. No user-facing functionality changed.

The poll loop stopped at the first page with any hit. Message hits can be indexed before the session title hit, so the title assertion failed under suite load. Poll until user, assistant, and title hits are all present.
@coderabbitai

coderabbitai Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: PyModel/pythinker-code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: adbd7039-8568-47ab-952f-d54c1bc9a8a2
📥 Commits

Reviewing files that changed from the base of the PR and between 9dbdc69 and 0848d85.

📒 Files selected for processing (1)
  • packages/agent-gateway/test/search/searchRoute.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The search-route test now continues polling until results include user, assistant, and title roles.

Changes

Search route test

Layer / File(s) Summary
Wait for expected result roles
packages/agent-gateway/test/search/searchRoute.test.ts
The polling loop now waits for results to include all three expected roles instead of stopping at any nonempty result set.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to 0848d

The search-route test now waits for the title result without masking a missing result. No actionable merge risk remains.

Architecture Summary

Architecture risk: 🔵 Low · up to 0848d

The change affects 1 system.

Changed systems: packages/agent-gateway

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/agent-gateway (api) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in packages/agent-gateway/test/search/searchRoute.test.ts: The polling loop now stops only after results contain all three expected roles (user, assistant, and title), rather than stopping as soon as any result appears.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely off-topic: it documents many changes that are not in the pull request summary, while omitting the template’s required sections for the actual search-route test change. Replace the unrelated session, workspace, terminal, and other change details with an accurate description of the test update. State the requirement or bug, explain that polling stopped before the title hit appeared, describe the change to w…
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title uses the required fix(test): prefix, starts with the imperative “wait,” is 58 characters, and describes the search-route test change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Resolution

Replace the unrelated session, workspace, terminal, and other change details with an accurate description of the test update. State the requirement or bug, explain that polling stopped before the title hit appeared, describe the change to wait for user, assistant, and title hits, and note that product behavior is unchanged. Add reproduction steps and root cause if treating this as a bug, and report relevant test coverage.

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pkg-pr-new

pkg-pr-new Bot commented Oct 3, 2026

Copy link
Copy Markdown
pnpm dlx https://pkg.pr.new/@pymodel/pythinker-code@0848d85
npx https://pkg.pr.new/@pymodel/pythinker-code@0848d85

commit: 0848d85

@elkaix
elkaix merged commit 8a76d79 into main Oct 3, 2026
27 checks passed
@elkaix
elkaix deleted the fix/search-route-title-wait branch October 3, 2026 01:43
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