Skip to content

fix: configure warnings gating, Windows tool resolution, and toast delivery#72

Merged
ualtinok merged 21 commits into
cortexkit:mainfrom
Zireael:fix/configure-warnings-and-path
Jun 1, 2026
Merged

fix: configure warnings gating, Windows tool resolution, and toast delivery#72
ualtinok merged 21 commits into
cortexkit:mainfrom
Zireael:fix/configure-warnings-and-path

Conversation

@Zireael
Copy link
Copy Markdown
Contributor

@Zireael Zireael commented May 29, 2026

Summary

After aft configure, users with formatting disabled still saw formatter/checker warnings, OpenCode could switch providers and hang when warnings fired on the first tool turn, and Windows installs of Biome/Ruff via npm were reported missing even when on PATH. This PR gates warnings on real edit settings, delivers them via idle-time toasts (not session chat), and shares robust PATH resolution across configure, format, and LSP.

Bugs, root causes, and fixes

Bug (user-visible) Root cause Fix
Formatter/checker “not installed” warnings when auto-format/validate is off (e.g. format_on_edit: false, no validate_on_edit) configure.rs always ran detect_missing_tools_for_languages from project config (biome.json, etc.) without checking format_on_edit / validate_on_edit Gate formatter warnings on format_on_edit or explicit formatter.<lang>; gate checker warnings on validate_on_edit (syntax/full) or explicit checker.<lang>. Integration tests cover the disabled-format path.
Provider switch + session hang after configure warnings deliverConfigureWarnings called sendIgnoredMessage with the resolved agent during the first tool turn (setTimeout(0)), which can emit ModelSwitched / AgentSwitched even with noReply: true Queue warnings per session; flush on session.idle. Default delivery is a 10s TUI/HTTP toast (configure_warnings_delivery: "toast"). Chat delivery omits agent to avoid switch side effects.
Global Biome (etc.) on Windows reported missing Weak tool resolution in configure.rs (no PATHEXT / node_modules/.bin / well-known dirs); duplicate logic vs format.rs New tool_path module: PATH walk, PATHEXT, well-known Windows dirs (npm, nodejs, pnpm, cargo, go, scoop). format.rs and lsp/registry.rs use it; configure uses format::tool_available_for_missing_warning().

Config note: There is no top-level formatters key. Use format_on_edit, formatter, checker, and optional configure_warnings_delivery (toast | log | chat, default toast). Documented in README, pi-plugin README, Zod schemas, and regenerated assets/aft.schema.json.

Test plan

  • cargo test — configure gating and tool_path-related unit/integration tests
  • OpenCode plugin tests — notifications (toast/log/chat), idle flush, orchestration
  • Manual: aft configure with format_on_edit: false → no formatter missing warnings unless formatter set
  • Manual: Windows PATH / %APPDATA%\npm Biome detected when global install present
  • Manual: OpenCode session — warnings appear as toast after idle, no provider switch on first edit


Summary by cubic

Gates configure-time missing-tool warnings on real edit settings and delivers them as a single toast after the session goes idle. Unifies cross‑platform tool discovery across configure/format/LSP to prevent false “not installed” alerts, especially on Windows.

  • Bug Fixes

    • Gate formatter warnings on format_on_edit or explicit formatter.<lang>; gate checker warnings on validate_on_edit (syntax/full) or explicit checker.<lang>.
    • Queue and batch warnings per session; flush on session idle. Default delivery is a 10s toast with HTTP fallback; chat delivery omits agent/model and records only successfully delivered warnings.
    • Shared tool_path for Windows/POSIX used by configure, format, and LSP: PATH resolution via which + manual walk; well‑known Windows dirs (npm, Node.js, pnpm, .cargo\bin, go\bin, scoop\shims, Go SDK). Checker detection accepts absolute paths; Ruff requires a format‑capable version.
    • CI/test stability: normalize CRLF in fixtures and golden corpus; path‑separator‑portable assertions; zombie‑safe process‑group kill; Windows .exe probing with F_OK; PATH‑isolated binary discovery; poll drain for bash completion; extend cross‑FS move_file timeout to 120s; fix e2e aft binary resolution on Windows; use correct project root in move_file test; use cross‑platform absolute path in semantic index dimension test; ignore .qartez/.
  • New Features

    • Added configure_warnings_delivery: toast (default), log, or chat. Documented in docs/config.md, added to packages/opencode-plugin and packages/pi-plugin schemas, and reflected in assets/aft.schema.json.

Written for commit ec3fcf1. Summary will update on new commits.

Review in cubic

Zireael added 3 commits May 24, 2026 02:23
…fy tool detection

- Make resolve_tool_uncached pub(crate) and add Windows well-known search paths
  (C:\Go\bin, %USERPROFILE%\.cargo\bin, etc.) to fix false negatives on Windows.
- Delegate configure.rs resolve_tool_uncached to format.rs to eliminate
  near-duplicate tool detection logic and ensure well-known path fallback
  applies at configure-time too.
- Add cfg!(windows) branch to install_hint("go") with Windows paths.
- Add .qartez/ to .gitignore (local code intelligence cache, not source).

Fixes cortexkit#47
Add shared tool_path resolution (PATH walk, PATHEXT, well-known dirs).
Gate missing formatter/checker warnings on format_on_edit and
validate_on_edit. Unify configure with format.rs availability checks.
Deliver missing-tool warnings on session idle via TUI/HTTP toast by
default (configure_warnings_delivery). Avoid createUserMessage during
first tool turn to prevent provider switch and hangs. Regenerate schema
and document the new option in README and pi-plugin config.
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

3 issues found across 18 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode-plugin/src/notifications.ts">

<violation number="1" location="packages/opencode-plugin/src/notifications.ts:695">
P2: Partial chat-delivery failures are incorrectly treated as full success, causing undelivered warnings to be marked as warned and never retried.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

formatConfigureWarningChat(warning),
{ includeAgent: false },
);
delivered = delivered || ok;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Partial chat-delivery failures are incorrectly treated as full success, causing undelivered warnings to be marked as warned and never retried.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/opencode-plugin/src/notifications.ts, line 695:

<comment>Partial chat-delivery failures are incorrectly treated as full success, causing undelivered warnings to be marked as warned and never retried.</comment>

<file context>
@@ -613,7 +638,63 @@ function formatConfigureWarning(warning: ConfigureWarning): string {
+      formatConfigureWarningChat(warning),
+      { includeAgent: false },
+    );
+    delivered = delivered || ok;
+  }
+  return delivered;
</file context>

Comment thread crates/aft/src/tool_path.rs
Comment thread crates/aft/src/tool_path.rs Outdated
Zireael added 4 commits May 29, 2026 23:14
Merge cortexkit/aft main. Resolve README and notifications conflicts:
keep upstream warnedStatus dedup and model pinning for normal messages;
configure warnings use toast idle delivery or per-message chat with
per-success recordWarning. Fix chat partial-delivery marking, Unix
executable checks in tool_path, and PATH test isolation.
detect_type_checker_go accepts absolute paths from tool_path resolution.
Add tests for chat partial dedup, omitted agent/model, toast without TUI,
and enqueue/idle flush lifecycle.
Align semantic integration tests with v0.32 degraded grep fallback, normalize CRLF in compress/structure fixture assertions for Windows bind mounts, poll bash promote drain and kill_all grandchild exit, resolve aft.exe in plugin e2e helpers, and normalize path suffix checks in search contract tests.
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 7 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/opencode-plugin/src/notifications.ts">

<violation number="1" location="packages/opencode-plugin/src/notifications.ts:695">
P2: Partial chat-delivery failures are incorrectly treated as full success, causing undelivered warnings to be marked as warned and never retried.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread crates/aft/src/lsp/child_registry.rs Outdated
Zireael and others added 13 commits May 30, 2026 04:57
Pass grandchild pid file via env instead of single-quoted shell interpolation. Use F_OK for .exe on Windows. Normalize helpers.ts to LF for CI biome lint.
Resolve merge conflicts:
- configure.rs: keep PR's should_warn_missing_* gating, discard main's
  resolve_tool_uncached/ruff_format_available duplicates
- format.rs: keep PR's resolve_on_path/probe_tool_in_dir extraction

Fix CI lint failures:
- notifications.ts: remove ! non-null assertion on warnings[0]
- bash.test.ts: remove unused pool destructure
- pi-plugin bash.test.ts: remove unused waitForFileText + readFile import
…ngs-and-path

# Conflicts:
#	crates/aft/tests/integration/semantic_test.rs
#	packages/opencode-plugin/src/__tests__/e2e/helpers.ts
- Restore upstream test bodies for semantic_search_falls_back_to_lexical* tests
- Remove unused assert_semantic_disabled_degraded_fallback helper
…ngs-and-path

# Conflicts:
#	crates/aft/src/lsp/registry.rs
#	crates/aft/tests/integration/aft_search_contract_test.rs
…ngs-and-path

# Conflicts:
#	crates/aft/src/commands/configure.rs
#	crates/aft/src/format.rs
- Set core.autocrlf=false to prevent line ending conversion
- Fix biome import ordering in index.ts (configure-warnings moved before hooks)
- Fix biome format in notifications.ts (collapse multi-line call to single line)
- Fix biome trailing newline in bash.test.ts
- Fix cargo fmt: remove 2 extra blank lines in format.rs
Golden files checked out on Windows CI carry \r\n line endings, but the
Rust process produces \n. Normalize both sides before byte-for-byte
comparison to prevent 107 false drift failures on Windows CI.
The cross-filesystem move test consistently times out at 60s on Linux CI
runners. Add send_with_timeout() helper and use 120s timeout for this
specific test.
…ions

The file-refresh-after-watcher-invalidation test used .ends_with("src/a.rs")
and .contains("src/a.rs") which fails on Windows where paths use backslashes.

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
…onfigure path

- Remove platform-conditional TARGET_DEBUG_BINARY_EXE/FALLBACK_BINARY_EXE
  from helpers.ts debugBinaryCandidates/fallbackBinaryCandidates (these
  constants were never declared, causing ReferenceError on Windows CI)
- Change move_file_cross_fs_copy_delete_failure configure from Path::new("/")
  to src_tmp.path() to match the actual test project root
- Update ARCHITECTURE.md and STRUCTURE.md to reflect current codebase state
@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented Jun 1, 2026

You're iterating quickly on this pull request. To help protect your rate limits, cubic has paused automatic reviews on new pushes for now—when you're ready for another review, comment @cubic-dev-ai review.

…test

Path::new('/') is not absolute on Windows, causing debug_assert failure.
Use std::env::temp_dir() which returns absolute path on all platforms.
@ualtinok
Copy link
Copy Markdown
Collaborator

ualtinok commented Jun 1, 2026

@cubic-dev-ai review

@cubic-dev-ai
Copy link
Copy Markdown

cubic-dev-ai Bot commented Jun 1, 2026

@cubic-dev-ai review

@ualtinok I have started the AI code review. It will take a few minutes to complete.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

0 issues found across 1 file (changes from recent commits).

Re-trigger cubic

@ualtinok ualtinok merged commit d92dfcc into cortexkit:main Jun 1, 2026
11 checks passed
ualtinok added a commit that referenced this pull request Jun 1, 2026
- Revert the max_depth(Some(8)) cap I added: 35745c8 deliberately removed
  it (and added configure_watcher_honors_deep_nested_aftignore, .aftignore
  10 levels deep). PR #72 already fixed the move_file timeout by configuring
  against a tempdir root instead of '/', so the cap is unnecessary and
  regressed the deep-ignore feature.
- Drop unused Path import in semantic_validation_test.rs and rustfmt
  helpers/mod.rs send_with_timeout (both from #72; its CI runs cargo test
  --workspace, not clippy -D warnings, so they slipped).
@Zireael
Copy link
Copy Markdown
Contributor Author

Zireael commented Jun 1, 2026

That was quick merge! :D

ualtinok added a commit that referenced this pull request Jun 1, 2026
ualtinok added a commit that referenced this pull request Jun 1, 2026
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