Add regression test coverage for merged PRs #13-#20 - #21
Merged
Conversation
added 2 commits
May 19, 2026 13:46
Adds targeted regression tests for each bug fix and behavioral guarantee introduced across the eight feature PRs. Total: 205 → 254 tests (+49). - background-tasks: 0 → 11 tests. Cover get()-evicts-on-read, truncation-by-chars on Completed/Failed, running tasks NOT evicted, shared state across clones (Arc<Mutex>), no-op on missing. - apply-patch: 8 → 16 tests. Multi-op stop-on-failure with prior ops staying applied, ambiguous-update rejection, 1MB create cap (incl. off-by-one), nested-dir creation, PatchOp deserialization. - glob-tool: 5 → 11 tests. Real-FS integration via TempTree: walks files, empty result returns empty string (not 'no files matched'), mtime sort with explicit path != CWD, respects .gitignore, regex metachars escaped, * doesn't cross directory boundaries. - plan-tools: 5 → 8 tests. Source-level regression test that plan_exit has no fs::write / PLAN.md side-effects, channel-unavailable + reply-dropped error paths. - web-tools: 7 → 12 / 4 → 7 tests. Wrap-width regression (long paragraph must wrap), input validation (empty/too many URLs), partial-field formatting in search results, 500-char snippet cap. - task-status: 6 → 9 tests. Completed task evicts after one read, wait=true returns on Failed, wait=true on missing errors promptly (bounded timeout guard against infinite-loop regression). - question-tool: 5 → 10 tests. Header → markdown ## heading, channel + reply error paths, default custom=true, multi-select comma-join. - agent-reminders: 0 → 5 tests. Extracted append_mode_reminder() pure fn so the reminder injection is unit-testable. Covers plan/review/ code modes, PLAN.md gate on code mode, unknown prompts pass through unchanged, section-separator prefix. Each regression test has a comment explaining the bug it guards against.
yogthos
added a commit
that referenced
this pull request
May 21, 2026
Follow-up to PR #111. Tier-2 items from the 23-bug audit batch: docs corrections and two small correctness/UX fixes. ## Docs - **#12 temperature** — CONFIG.md claimed "parsed but not currently applied". Actually applied since PR #105 with a clamp warning. Rewrote the cell. - **#13 --api-key** — flag existed but neither README nor CONFIG.md mentioned it. Added a Quick-start example noting the process-list visibility caveat. - **#14 acp_host/acp_port** — CONFIG.md documented both keys but the CLI flags were intentionally removed (stdio-only transport). Removed both from the keys table + ACP section. - **#6 tools** — `Config::tools` (per-tool enable map) was fully wired in code but undocumented. Added a row to the keys table covering `tools.websearch` and `tools.webfetch`. - **#21 find_callers** — README claimed "word-boundary regex" but the impl uses the tree-sitter symbol index. Updated to reflect actual behavior; the user-visible word-boundary semantics are preserved. ## Code - **#16 semantic index skip_dir** — `SymbolIndex::find_callers` filter had its own hardcoded `matches!(name, "node_modules" | "target" | ".git" | "__pycache__")` while the rest of the codebase uses `agent::tools::is_skip_dir`. Switched to the shared helper so future additions stay in lockstep. - **#18 context::load_file** — silently swallowed `read_to_string` errors via `.ok()`. A permission-denied AGENTS.md looked identical to a missing file. Now emits a stderr warning naming the path + reason; still returns None so callers' behavior is unchanged. 725 plugin / 599 default pass. All build profiles clean. ## Remaining audit items (deferred to feature work) - **#8 LSP no crash restart**: needs broken-pipe IO error handling + exponential backoff. Touches manager state machine. - **#10 task tool fire-and-forget**: needs timeout + cleanup coordination via JoinHandle tracking. - **#25 MCP no reconnection**: similar architectural concern to #8. - **#27 LSP didClose**: client lifecycle hook missing. - **#29 token estimation len/4**: needs per-provider usage extraction (Phase 6 work). - **#5 MCP shutdown**: rmcp Drop semantics need verification. - **#38/39/40 semantic test gaps**: get_symbol_body untested, list_symbols kind_filter untested, find_definition test vacuous. Sat down to add but each requires a fixture build. Together with PR #111 (10 code fixes), 17 of the 23 verified items are now shipped. Remaining 6 are architectural or test-infrastructure work better tackled as discrete PRs. Co-authored-by: Yogthos <yogthos@gmail.com>
allen-munsch
pushed a commit
to allen-munsch/dirge
that referenced
this pull request
Jun 3, 2026
…de#112) Follow-up to PR dirge-code#111. Tier-2 items from the 23-bug audit batch: docs corrections and two small correctness/UX fixes. ## Docs - **dirge-code#12 temperature** — CONFIG.md claimed "parsed but not currently applied". Actually applied since PR dirge-code#105 with a clamp warning. Rewrote the cell. - **dirge-code#13 --api-key** — flag existed but neither README nor CONFIG.md mentioned it. Added a Quick-start example noting the process-list visibility caveat. - **dirge-code#14 acp_host/acp_port** — CONFIG.md documented both keys but the CLI flags were intentionally removed (stdio-only transport). Removed both from the keys table + ACP section. - **dirge-code#6 tools** — `Config::tools` (per-tool enable map) was fully wired in code but undocumented. Added a row to the keys table covering `tools.websearch` and `tools.webfetch`. - **dirge-code#21 find_callers** — README claimed "word-boundary regex" but the impl uses the tree-sitter symbol index. Updated to reflect actual behavior; the user-visible word-boundary semantics are preserved. ## Code - **dirge-code#16 semantic index skip_dir** — `SymbolIndex::find_callers` filter had its own hardcoded `matches!(name, "node_modules" | "target" | ".git" | "__pycache__")` while the rest of the codebase uses `agent::tools::is_skip_dir`. Switched to the shared helper so future additions stay in lockstep. - **dirge-code#18 context::load_file** — silently swallowed `read_to_string` errors via `.ok()`. A permission-denied AGENTS.md looked identical to a missing file. Now emits a stderr warning naming the path + reason; still returns None so callers' behavior is unchanged. 725 plugin / 599 default pass. All build profiles clean. ## Remaining audit items (deferred to feature work) - **dirge-code#8 LSP no crash restart**: needs broken-pipe IO error handling + exponential backoff. Touches manager state machine. - **dirge-code#10 task tool fire-and-forget**: needs timeout + cleanup coordination via JoinHandle tracking. - **dirge-code#25 MCP no reconnection**: similar architectural concern to dirge-code#8. - **dirge-code#27 LSP didClose**: client lifecycle hook missing. - **dirge-code#29 token estimation len/4**: needs per-provider usage extraction (Phase 6 work). - **dirge-code#5 MCP shutdown**: rmcp Drop semantics need verification. - **dirge-code#38/39/40 semantic test gaps**: get_symbol_body untested, list_symbols kind_filter untested, find_definition test vacuous. Sat down to add but each requires a fixture build. Together with PR dirge-code#111 (10 code fixes), 17 of the 23 verified items are now shipped. Remaining 6 are architectural or test-infrastructure work better tackled as discrete PRs. Co-authored-by: Yogthos <yogthos@gmail.com>
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.
Summary
Targeted regression tests for the bugs found during review of the eight feature PRs. 205 → 254 tests (+49). Each regression test has a comment naming the specific bug it guards.
Per-feature additions
background.rsget()evicts on read;update()truncates Completed/Failed by chars (not bytes — emoji round-trip); running tasks NOT evicted; clones share stateapply_patch.rsglob.rsTempTreewalker; empty result → ""; mtime sort with explicit path ≠ CWD; respects.gitignore; regex metachars escaped;*bounded to single segmentplan.rsplan_exithas nofs::write/PLAN.md side-effects; channel-unavailable + reply-dropped pathswebfetch.rshtml.len()for width); empty/too-many URL validation;WebFetchArgsdefaultmax_chars=3000websearch.rsWebSearchArgsdefaultnum_results=10; 500-char snippet captask_status.rswait=truereturns on Failed;wait=trueon missing task errors promptly (boundedtokio::time::timeoutguards against infinite-loop regression)question.rs##heading; channel error paths; defaultcustom=true; multi-select comma-joinbuilder.rs(agent-reminders)append_mode_reminder()pure fn. Covers plan/review/code modes,PLAN.mdgate on code mode, unknown prompts pass throughRefactor
build_agent_innerhad the mode-reminder switch inlined and untestable. Extracted intoappend_mode_reminder(preamble, prompt_name, plan_exists)— pure function, no I/O — and tested directly.Test plan
cargo buildcargo test --bin dirge -- --skip plugin→ 254 passed, 0 failedcargo fmt --check