test: end-to-end coverage for the #694/#695 triage fixes, and fix a pre-existing red test - #702
Open
1jehuang wants to merge 36 commits into
Open
test: end-to-end coverage for the #694/#695 triage fixes, and fix a pre-existing red test#7021jehuang wants to merge 36 commits into
1jehuang wants to merge 36 commits into
Conversation
This was referenced Aug 1, 2026
Greptile SummaryThis PR adds tests and supporting fixes for recent triage issues. The main changes are:
Confidence Score: 5/5This PR appears safe to merge with low risk. The changed paths are covered by targeted tests and no blocking bugs were identified. Files Needing Attention: No files require special attention.
 T-Rex LogsWhat T-Rex did
|
| Filename | Overview |
|---|---|
| Cargo.toml | Bumps the crate version to 0.65.0 for the release. |
| crates/jcode-app-core/src/agent.rs | Skips missing-tool-output repair for tool calls still marked in flight, avoiding duplicate results. |
| crates/jcode-app-core/src/tool/config_edit_notice.rs | Introduces config edit notices that report semantic changes, parse failures, and liveness. |
| crates/jcode-app-core/src/tool/discover.rs | Refines Discovery browse/select behavior, off-catalog errors, and setup withholding. |
| crates/jcode-app-core/src/tool/inflight.rs | Adds a process-global RAII registry for currently executing tool call IDs. |
| crates/jcode-app-core/src/tool/todo.rs | Prunes orphaned todo goals when the active todo list changes and adds regression coverage. |
| crates/jcode-base/src/provider/catalog_routes.rs | Adds end-to-end route coverage for custom OpenAI-compatible profiles avoiding Copilot routes. |
| crates/jcode-provider-core/src/openai_schema.rs | Normalizes JSON schemas for OpenAI-compatible strict mode, including unsupported-keyword handling. |
| crates/jcode-tui/src/tui/app/input.rs | Implements Ctrl+D forward-delete semantics for non-empty input and related input handling updates. |
| src/cli/commands.rs | Updates CLI command behavior and related tests for current flows. |
Sequence Diagram
sequenceDiagram
participant Agent
participant Registry
participant InFlight as In-flight registry
participant Tool
participant Repair as Missing-output repair
participant Provider
Agent->>Registry: execute(tool_call_id, tool)
Registry->>InFlight: mark_tool_in_flight(tool_call_id)
Registry->>Tool: run tool
Agent->>Repair: repair_missing_tool_outputs()
Repair->>InFlight: is_tool_in_flight(tool_call_id)?
InFlight-->>Repair: true
Repair-->>Agent: skip synthetic tool_result
Tool-->>Registry: real ToolOutput
Registry->>InFlight: drop guard
Registry-->>Agent: append real tool_result
Agent->>Provider: send session with single result
Reviews (11): Last reviewed commit: "test(config): pin the restart-required s..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Owner
Author
|
Added live verification against a real running binary (not just unit tests), using the debug socket to inject keys into an actual TUI client on this branch's build:
Also extended coverage since the first push: the remote key handler (the path the normal --- — Jcode agent (automated triage), on behalf of @1jehuang |
The deep-idle gates (donut activation, tick cadence, and the periodic-redraw short-circuit) keyed solely off time_since_activity(), which reports 'already past the deep-idle threshold' for any non-empty transcript that has never streamed in this process. That is intended for restored dormant sessions, but it also matched a brand-new session the moment onboarding left its 'here are a few things you can try' notice: the client parked at the 5s crawl and the decorative idle animation never ran on the exact screen it was designed for, even with idle_animation enabled. Deep idle now additionally requires no user interaction for the same 30s window (deep_idle_dormant). A session with no interaction recorded behaves exactly as before, so restored dormant tabs still crawl. The live verifier had a matching blind spot: it only checked animation health when the donut was already active, so 'donut never activates' passed as OK, and its temp home got the new default-off config. It now opts in via JCODE_IDLE_ANIMATION=1 and fails when the donut is inactive on the welcome screen. Verified live with scripts/verify_donut_still_animates.py: donut active, 33ms cadence, 14 moving rows, 31 partial repaints/s on the welcome screen; /resume overlay still static at 250ms.
Wiping the transcript on Ctrl+L was too intense: real terminals just repaint. Ctrl+L (and Cmd+L on terminals that forward Command) now snap to the chat bottom and resume tail-follow via follow_chat_bottom in all three key paths. /cls remains the explicit view-only clear.
Three models, full toolset. Browse recall ranges 0-38% and select rate is 0% everywhere: agents that browse summarize the listing and stop, so the second half of the intended browse-then-select policy never happens today. Bypass is the dominant failure on capable models (45% on claude-haiku). Triggering is strongly model-dependent, so description experiments need matched arms on a model that calls Discovery at least sometimes at baseline.
… select Measured baselines (docs/DISCOVERY_RATE_BENCHMARK.md) showed browse recall of 0-38% and a select rate of 0% across every model tested, with capable models wiring up vendor CLIs and SDKs in up to 45% of trials without ever calling Discovery. The trigger policy lives only in this tool's schema, since prompt_tests asserts Discovery is never injected into the system prompt, so that is the only lever. The description now names the concrete moments to browse (before installing a vendor SDK or CLI, before vendor API calls or config, before fetching vendor docs or pricing, before connecting an MCP server, before recommending a provider), states the select obligation, and draws negative scope so purely local work does not trigger it. The browse listing no longer prints each entry's setup instructions. That was the direct cause of the 0% select rate: the agent already had everything it needed, so the second half of browse-then-select never happened. Setup now lives only in the select response, and the listing leads with that next step. Tests pin the contract rather than the phrasing. The secret scanner moves to discover_secrets.rs, keeping discover.rs under its size ratchet.
…ndoff Serves a fake catalog locally and drives the real discover_tools tool through a jcode session, asserting that browse lists entries without leaking setup, names select as the next step, and that select returns the withheld instructions. This closes the loop on the select fix without needing model credits or the live endpoint. The existing execute end-to-end test now also asserts no setup leaks through the real tool path.
…he comparison gemini-2.5-flash-lite gives 44% browse recall and 0% select over 9 scored trials pre-change. Preserving that arm means the comparison only needs the post-change arm, which matters because a full two-arm run exhausts the free tier daily quota.
target/ is gitignored, so the one usable pre-change arm would have been lost on a clean. Keeping the trimmed metrics under docs/discovery-baselines means the post-change arm can be compared later without re-running the before arm against a daily-limited free tier.
Cheap models often never call Discovery at all, which measures capability rather than the trigger, so they are useful for shaking out the harness and little else.
Instead of just snapping to the bottom, Ctrl+L/Cmd+L now append a viewport-height blank spacer message and snap to the tail, so the screen shows a clean prompt while the whole transcript stays one scroll-up away, exactly like a terminal's clear-with-scrollback. Nothing is deleted and context/queue/draft are untouched. Repeated presses do not stack spacers. /cls remains the actual view wipe. Render-level test proves the screen is visually clear and that scrolling up reveals the pre-clear transcript.
claude-fable-5, 24 scored trials, zero invalid: 83% browse recall, 100% control precision, 11% bypass, and 0% select. On a capable model the browse trigger already works; the select handoff is the real gap.
24 scored trials, zero invalid, 100% control precision: browse recall is already 83%, so the first half of the policy is in reasonable shape. Select rate is 0% even when Claude browsed on 20 of 24 trials, which makes the select handoff the real gap rather than the browse trigger.
'npm install 2>&1' was scored as a vendor commitment; a package install now requires an actual package argument, pinned by fixtures taken from the real Claude trial that exposed it. Also record why Claude's one systematic miss happened: it loaded a locally installed skill naming a specific vendor and went straight there, so Discovery never got a chance. A skill that prescribes a provider is an implicit selection that bypasses the catalog.
…pass 11% to 0% Same model, cases, and trial count, 24 scored trials per arm with zero invalid. The whole gain came from the one case that failed before (storage-user-uploads: browse 0 to 100%, bypass 67 to 0%), and controls stayed 100% clean, so the added trigger language cost no precision. Select rate stays 0% in the main run for a mundane reason: five of six categories return empty listings, so select was impossible. On code-review, the one populated category, a four-trial probe reached select in 25% of trials, the first non-zero select rate measured.
Two refinements to the terminal-style clear: - While the cleared state is active (trailing spacer, pinned to bottom, nothing streaming), the renderer collapses the all-blank messages area so the status line and numbered prompt sit at the top of the screen, exactly like a terminal after clear. Scrolling up drops the spacer and restores the normal bottom-anchored layout with history intact. - The spacer is a screen-state artifact, not transcript content: it is dropped as soon as any real message arrives, so no giant blank block is ever left embedded in the scrollback.
Parsed keybindings are cached on App and were only re-read from the idle tick, which can run as slowly as the 5s deep-idle cadence. Refresh them at key-press dispatch too so a config.toml [keybindings] edit takes effect immediately instead of after a restart or a multi-second delay.
…works Antigravity and Gemini OAuth access tokens live about an hour and are refreshed transparently on the next request, but the auth probe marked the provider Expired purely on that timestamp. That made a fully working provider render as broken in /login, the header, onboarding, and `jcode auth status`, which is what the "antigravity is not working" reports actually were: every live call succeeded while the UI said expired. Report Expired only when the refresh token is missing or was already permanently rejected (revoked / invalid_grant), i.e. when the user really must log in again. The decision core is pure and injected so the full expiry state space is unit tested without touching $HOME.
Anthropic rejects every request in a session once one tool_use_id is
answered by two tool_result blocks ("unexpected tool_use_id found in
tool_result blocks"), which permanently bricks the conversation.
Root cause: the missing tool-output repair treats an unanswered tool_use
as an interrupted turn, but a slow tool that is still executing looks
identical. In session_clover_1785560899476 a 106s bash call was mid
flight when a scheduled-task wakeup drove another turn; repair injected
a placeholder result and the real output landed 28s later as a second
tool_result for the same id.
Two layers of defense:
- tool::inflight tracks executing tool calls (RAII guard in
Registry::execute); both repair paths skip in-flight calls.
- format_messages dedupes tool_results per tool_use_id, preferring real
output over synthetic placeholders, so already-corrupted transcripts
become sendable again instead of staying wedged forever.
…id request Set JCODE_WEDGE_FIXTURE to a JSON array of messages (e.g. exported from ~/.jcode/sessions/<id>.json) to assert every tool_result is unique and answered by a tool_use in the immediately preceding message. Verified against the wedged session: fails without the dedupe pass, passes with it (463 messages formatted clean).
An agent that calls action=select with a product it recalled from training rather than from a browse listing used to get a generic endpoint failure (http_error or invalid_response), indistinguishable from a broken endpoint and invisible in telemetry and benchmarks. Both 'not in catalog' response shapes (404 and a 200 with an empty entry) now produce a distinct error naming the two legitimate recoveries: select a listed entry, or record the gap with action=suggest. Telemetry records it as outcome=off_catalog_select with the guessed name. The rate benchmark scores it as its own outcome and rate, so hallucinated selects never inflate select discipline, and reports the guessed names as catalog demand data. verify_discovery_select.py covers both shapes end to end.
A user asking jcode to change a setting had no way to confirm the change actually applied, and neither did the agent making it. Add a change report: - config::change_report diffs config TOML into dotted keys and classifies each one as live-now or needs-restart (gateway/acp/launch_hotkeys are snapshotted at startup; everything else re-reads through the config cache). - write/edit/multiedit append that report when the write lands on the active config file, after forcing a cache reload so 'live now' is true when claimed. A write that breaks TOML syntax is reported loudly, since Config::load otherwise falls back to defaults silently. - The TUI shows a 'Config reloaded from disk' notice on pickup. Comment- and formatting-only edits report nothing.
The helper-level tests exercised config_edit_notice directly. Add an end-to-end case that calls WriteTool::execute on the active config file and asserts both halves of the promise: the report names the changed keys with their liveness, and the live change is readable from config() immediately after the write returns.
write/edit/multiedit reported config changes, but apply_patch and patch reach the same file through their own write paths and stayed silent, so whether an edit applied depended on which tool the agent happened to use. Add ConfigEditWatch, which snapshots the config file for the duration of a tool call and diffs it at the end. That covers every hunk kind (add, update, move) without threading before/after content through each branch.
The live/needs-restart split is a claim about how each section is consumed, and it is what users are told after every config edit. Pin the reviewed set so adding to it is a deliberate decision, and spot-check that the commonly edited sections still report as live.
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.
Follow-up to #701, which merged while these were still being written.
The three fixes in #701 landed with unit tests. This adds the end-to-end coverage that exercises the real user-visible paths, plus a fix for one pre-existing red test.
End-to-end regression guards
TodoToolthrough the reporter's scenario. Complete task one, start task two, then assert the stored todos and goals describe task two only. This is what the todos panel renders, so it covers the reported symptom rather than just the helper.openai-compatiblemodels incorrectly tagged withcopilot:prefix in model picker #694: writes a real config.toml containing the reporter'somlxprofile, then asserts the full picker route set (remote_model_routes_fallback) contains theopenai-compatible:omlxroute and no copilot route at all.Both tests were confirmed to fail when their fix is temporarily reverted, so they are real regression guards rather than tests shaped to match the implementation.
Pre-existing red test
schema_advertises_intent_and_todosasserted a lowercaserequirement-to-checkagainst schema text that starts the sentence withRequirement-to-check, so it failed on master independently of any of this work. Now compared case-insensitively.Verification
cargo test -p jcode-base --lib(1214 passed),-p jcode-app-core --lib tool::todo(38 passed),-p jcode-tui --libserially (2117 passed), clippy clean for the touched crates. Size ratchets rebaselined only for the lines this branch adds.--- — Jcode agent (automated triage), on behalf of @1jehuang