Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdd Nightly/Cranelift workspace config, new proc‑macro test crate, capability/UTF‑8 filesystem helpers, socket endpoint parsing and secure Unix preparation, refactor refactor/dispatch request-building and execution flow, and widespread import grouping plus one-line body compaction across the workspace. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as "Caller"
participant Parser as "SocketEndpoint::from_str"
participant Prep as "preparation::prepare_filesystem"
participant CapFS as "cap_std::fs::Dir / OS"
Caller->>Parser: provide socket URL string
Parser->>Caller: return SocketEndpoint or parse error
alt endpoint == Unix
Parser->>Prep: prepare_filesystem(path)
Prep->>CapFS: probe existing prefix, check symlinks & metadata
CapFS-->>Prep: metadata / exists result
Prep->>CapFS: create missing dirs and harden perms
CapFS-->>Prep: create/harden result
Prep-->>Parser: Ok(()) or SocketPreparationError
else endpoint == TCP
Parser-->>Caller: no filesystem preparation required
end
Parser-->>Caller: final SocketEndpoint + prep outcome
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-build-util/src/lib.rs Comment on file pub fn write_man_page(data: &[u8], dir: &Utf8Path, page_name: &str) -> io::Result<Utf8PathBuf> {
// Find the first existing ancestor directory
let mut existing_ancestor = dir;
while !existing_ancestor.as_str().is_empty() && existing_ancestor.parent().is_some() {
if Dir::open_ambient_dir(existing_ancestor, cap_std::ambient_authority()).is_ok() {
break;
}
existing_ancestor = existing_ancestor.parent().unwrap_or(existing_ancestor);
}
// If no existing ancestor found, use current directory
if existing_ancestor.as_str().is_empty() {
existing_ancestor = Utf8Path::new(".");
}
// Open the existing ancestor directory
let base_dir = Dir::open_ambient_dir(existing_ancestor, cap_std::ambient_authority())?;
// Get the relative path from the ancestor to the target
let relative_path = dir.strip_prefix(existing_ancestor).unwrap_or(dir);
// Create all intermediate directories
if !relative_path.as_str().is_empty() {
create_dir_all_cap(&base_dir, relative_path)?;
}
// Open the target directory
let target_dir = if relative_path.as_str().is_empty() {
base_dir
} else {
base_dir.open_dir(relative_path)?
};
let destination = page_name;
let tmp = format!("{page_name}.tmp");
// Write to temp file
target_dir.write(&tmp, data)?;
// Try atomic rename
match target_dir.rename(&tmp, &target_dir, destination) {
Ok(()) => {}❌ New issue: Complex Method |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-syntax/src/pattern.rs Comment on file use crate::language::SupportedLanguage;
use crate::metavariables::{extract_metavar_name, placeholder_for_metavar};
use crate::parser::{ParseResult, Parser};
use crate::{❌ New issue: String Heavy Function Arguments |
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. crates/weaverd/src/dispatch/observe/get_card_tests.rs Comment on file fn handle_returns_semantic_success_with_enrichment_and_rewritten_provenance(
temp_dir: TempDir,
) -> Result<(), String> {❌ New issue: Code Duplication |
Owner
Author
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. crates/weaver-cards/src/graph_slice/parse.rs Comment on file fn apply_max_cards_flag<'a, I>(&mut self, iter: &mut I) -> Result<(), GraphSliceError>
where
I: Iterator<Item = &'a String>,
{
let raw = require_arg_value(iter, Flag::MaxCards)?;
let max_cards = parse_u32(raw)?;
self.budget = self.budget.with_max_cards(max_cards);
Ok(())
}❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai please investigate the cause of the following build failure in the Linux x86_64 release dry run and identify a fix, providing an AI agent prompt for the fix: |
This comment was marked as resolved.
This comment was marked as resolved.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9f2cd29ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 50
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cargo/config.toml:
- Around line 7-16: Add a "Toolchain prerequisites" section to CONTRIBUTING.md
(or README) that documents the required components referenced by the .cargo
config: that the project needs Nightly Rust for -Zthreads and
cranelift/codegen-backend, instructions to install the cranelift component via
rustup (rustc-codegen-cranelift-preview), and that clang and the mold linker
must be installed for the x86_64 and aarch64 targets referenced by the target.*
blocks and rustflags (e.g., codegen-backend = "cranelift", rustflags = ["-C",
"link-arg=-fuse-ld=mold"]). Ensure the note includes exact commands to install
Nightly and the rustup component and a brief hint about expected opaque build
failures if missing.
- Around line 1-2: The [build] rustflags entry currently hardcodes "-Zthreads=8"
which can hurt smaller machines; remove the hardcoded "-Zthreads=8" from the
rustflags setting in the .cargo/config.toml or replace it with a configurable
mechanism (e.g., omit the flag so rustc uses its default, or load rustflags from
an included config/env so users can override); update documentation
(CONTRIBUTING.md) to call out the threads assumption if you choose to keep a
tunable default.
In @.github/workflows/ci.yml:
- Around line 45-48: Update the workflow step named "Install
rustc-codegen-cranelift" so the rustup command installs the cranelift component
for the same nightly as rust-toolchain.toml: replace the hardcoded "--toolchain
nightly-2025-09-18" in the "rustup component add rustc-codegen-cranelift
--toolchain ..." invocation with "--toolchain nightly-2026-03-26" or,
preferably, read the toolchain value from rust-toolchain.toml into a variable
and use that variable here to keep them in sync.
In @.gitignore:
- Around line 6-10: Update the .gitignore patterns that currently list "*.swp",
"*.swo", "*.swn", "*~", and "*.un~" so they match Vim's actual swap/backup
filenames which begin with a dot (e.g., ".filename.swp"); replace them with
dot-prefixed patterns such as ".*.swp", ".*.swo", ".*.swn", ".*~" and ".*.un~"
to ensure Vim temp files are ignored (locate these entries by the existing
patterns "*.swp", "*.swo", "*.swn", "*~", and "*.un~").
In `@crates/sempai-core/src/tests/capture_tests.rs`:
- Around line 4-10: Remove the import of
weaver_test_macros::allow_fixture_expansion_lints and the
#[allow_fixture_expansion_lints] attribute on the fixture; instead add a
narrowly scoped expect attribute on the fixture function sample_span:
#[expect(unused_braces, reason = "rstest fixture expansion introduces braces not
authored in this module")]. Ensure the fixture signature and body (fn
sample_span() -> Span { Span::new(0, 5, LineCol::new(0, 0), LineCol::new(0, 5))
}) remain unchanged and do not introduce any #[allow(...)] usages.
In `@crates/sempai-yaml/src/project_depends_on.rs`:
- Around line 51-52: Replace the backslash line-continuated string literal in
project_depends_on.rs (the message starting with
"`r2c-internal-project-depends-on` must define string `namespace` and `package`
fields") with a concat!() call to join the parts; locate the error/message
construction (where that string is defined/returned) and change it to concat!(
"...first part...", "...second part..." ) so there are no escaped-newline
continuations while preserving the exact message content and spacing.
- Around line 61-62: The string literal error message
"`r2c-internal-project-depends-on` must define string `namespace` and `package`
fields" currently uses backslash line continuation; replace that continued
literal with a concat!() call (following the pattern used earlier in the file
around lines 51–52) so the message is built via concat!() instead of using a
trailing backslash; update the string expression where that exact message occurs
in project_depends_on.rs accordingly.
- Around line 71-72: Replace the backslash-continuated string literal for the
error message "`r2c-internal-project-depends-on` must define string `namespace`
and `package` fields" with a concat!() call; locate the string in
project_depends_on.rs and change the escaped newline usage to something like
concat!("`r2c-internal-project-depends-on` must define string `namespace` ",
"and `package` fields") so the message is built via concat!() instead of using a
backslash continuation.
In `@crates/sempai-yaml/src/raw.rs`:
- Around line 3-6: Update the module doc comment in raw.rs to use the Oxford
“-ize” spelling: replace the word "deserialised" in the top doc comment (the
triple-slash comment block that describes conversion into the `model` layer via
`TryFrom`) with "deserialized" so the docs conform to the repository's preferred
spelling.
In `@crates/sempai-yaml/src/tests/behaviour.rs`:
- Around line 6-7: Remove the use statement importing
allow_fixture_expansion_lints and replace every occurrence of
#[allow_fixture_expansion_lints] with one or more narrowly scoped #[expect(...)]
attributes that match the specific lints the macro was previously silencing
(e.g. #[expect(unused_braces)] and any other exact lint names the macro
injected); apply these #[expect(...)] attributes directly on the test functions
or local items that need them so you avoid any global #[allow(...)] and then
delete the now-unused use weaver_test_macros::allow_fixture_expansion_lints;
import.
In `@crates/sempai/src/tests/behaviour.rs`:
- Around line 6-7: Remove the macro-based import and attribute: delete the `use
weaver_test_macros::allow_fixture_expansion_lints;` import and remove any
`#[allow_fixture_expansion_lints]` placed on the fixture, and instead add a
narrowly scoped expect attribute directly on the fixture like
`#[expect(fixture_expansion_lints, reason = "brief reason why this fixture
triggers the lint")]` so the suppression is explicit and scoped to that fixture;
ensure you replace every occurrence (the fixture attribute and the import)
referenced in this file.
In `@crates/sempai/src/tests/engine_tests.rs`:
- Around line 42-45: The test uses backslash-escaped newline continuations
inside long YAML string literals passed to engine.compile_yaml which violates
the repo guideline; replace those backslash-continued literals with concat!()
calls to join the string pieces (e.g., update the engine.compile_yaml
invocations in the tests where the long YAML is constructed—the call site using
"rules:\n - id: bad\n..." and the other occurrences around lines referenced—to
build the multi-line YAML with concat!(...) instead of using a trailing
backslash-escaped line break).
In `@crates/weaver-build-util/src/lib.rs`:
- Around line 182-190: The function find_existing_ancestor returns "." without
ever testing the filesystem root; update its loop to probe ancestors until the
true root is checked by replacing the current while condition with an
unconditional loop that checks Dir::open_ambient_dir(candidate, ...) and then
obtains candidate.parent() with pattern matching (let Some(parent) =
candidate.parent() else { break; }) and breaking if parent == candidate before
setting candidate = parent; this ensures the root (or drive root on Windows) is
tested before falling back to Utf8Path::new(".").
In `@crates/weaver-cards/src/detail.rs`:
- Around line 62-63: The backslash-continued string literal `"unknown detail
level: {}; expected one of: minimal, signature, structure, semantic, \ full"`
should be replaced with a compile-time concatenation using concat!() to meet
Rust style guidelines; find the occurrence of that exact message (used where the
code reports an invalid detail level) and change it to something like
concat!("unknown detail level: {}; expected one of: minimal, signature,
structure, semantic, ", "full") so there is no backslash continuation and the
message is assembled at compile time.
In `@crates/weaver-cards/src/graph_slice/response.rs`:
- Around line 253-254: The help text currently uses a backslash-continued string
literal ("no_symbol_at_position, position_out_of_range, unsupported_language,
not_yet_implemented, backend_unavailable")—replace that escaped newline style
with a concat!() invocation that joins the pieces at compile time; locate the
string in response.rs (the help text constant/assignment containing
"no_symbol_at_position, position_out_of_range, unsupported_language,
not_yet_implemented, backend_unavailable") and rewrite it as
concat!("no_symbol_at_position, position_out_of_range, ", "unsupported_language,
not_yet_implemented, ", "backend_unavailable") so it follows the repository
guideline and preserves the same resulting string.
In `@crates/weaver-cards/src/tests/behaviour.rs`:
- Around line 181-183: The .expect call on parsed.pointer(&pointer) removed
useful context; restore a formatted panic message that shows the missing key and
the parsed JSON by replacing the current expect("expected JSON to contain key")
with either an expect! that includes the key (e.g., expect(&format!("expected
JSON to contain key '{}': {}", key.as_str(), parsed))) or revert to
unwrap_or_else(|| panic!(...)) that panics with the key and parsed.to_string();
update the reference around parsed.pointer(&pointer) and the variables
pointer/key/actual accordingly so test failures include both the missing key and
the parsed JSON.
In `@crates/weaver-cards/src/tests/extractor/whitespace_stability_tests.rs`:
- Around line 12-13: The test uses a backslash-continued string literal for the
variable named source that contains the doc comments and the fn greet
definition; replace that backslash-escaped multi-line literal with concat!(...)
of separate quoted segments (including explicit "\n" where needed) so each
original line becomes its own string literal concatenated by concat!; update the
other occurrence mentioned (lines 20–22) similarly; locate the source literal in
whitespace_stability_tests (the test that contains the "/// Greets callers." doc
comment and fn greet) and replace the backslash-continued string with a
concat!() expression preserving the same final string content.
In `@crates/weaver-cli/src/lib.rs`:
- Around line 421-423: The helper write_error_and_fail currently discards the
Result from writeln! using "let _ =", which breaks a deny lint; change the call
to explicitly consume the Result (e.g., call .ok() on the writeln! result) so
the write error is intentionally ignored without using underscore discard.
Update the writeln! invocation inside write_error_and_fail to use .ok() to
satisfy the let_underscore_must_use rule while keeping the function behavior the
same.
In `@crates/weaver-cli/src/lifecycle/monitoring.rs`:
- Around line 265-269: The function next_poll_interval currently maps a past
deadline to POLL_INTERVAL causing an extra sleep; change the fallback so that
when deadline.checked_duration_since(Instant::now()) returns None you return
Duration::ZERO instead of POLL_INTERVAL. Update next_poll_interval to use
Duration::ZERO as the map_or/default value and keep the
remaining.min(POLL_INTERVAL) branch unchanged so it returns the lesser of
remaining and POLL_INTERVAL when the deadline is still in the future.
In `@crates/weaver-cli/src/output/render.rs`:
- Around line 200-209: The function write_render_line currently uses let _ =
output.write_fmt(args) which violates the let_underscore_must_use lint because
write_fmt returns a must-use Result; update write_render_line to explicitly
consume the Result returned by fmt::Write::write_fmt (e.g., call .ok() on the
returned Result) so the return value is discarded intentionally while keeping
the function signature and behavior unchanged; locate the write_render_line
function and replace the let _ = ... line accordingly.
In `@crates/weaver-cli/src/tests/support/mod.rs`:
- Around line 372-374: Remove the repo-wide suppression by deleting
#[allow_fixture_expansion_lints] on the fixture and instead add a narrowly
scoped lint expectation for the specific lint the macro expansion triggers:
replace that attribute with #[expect(unused_braces, reason = "fixture macro
intentionally expands to extra braces")] applied to the fixture function world
(the fn world -> RefCell<TestWorld>), so only the unused_braces lint is
suppressed with a documented reason rather than a broad allow.
In `@crates/weaver-config/Cargo.toml`:
- Line 35: The crate is pinning cap-std to "3.0" causing a mismatch with the
workspace; update the dependency declaration for cap-std in this Cargo.toml to
source it from the workspace (use the workspace override syntax) so it aligns
with the workspace's cap-std = "4.0" and avoids lockfile/version drift—replace
the direct version entry for cap-std with the workspace-sourced declaration
(cap-std = { workspace = true }) in this file.
In `@crates/weaver-config/src/socket/preparation.rs`:
- Around line 94-97: The code currently calls create_socket_directory(parent)
before running Unix hardening, allowing attacker-controlled paths to create
entries; change the flow in prepare_socket (or wherever create_socket_directory
and ensure_secure_directory are invoked) to first split parent into an
existing_prefix (the longest ancestor that already exists) and a missing_suffix
(the remaining path components), run the security/validation checks (reuse
ensure_secure_directory or implement a helper like validate_existing_prefix)
against existing_prefix to detect symlinks/ownership/traversal, and only after
validation create each component of missing_suffix one-by-one under
existing_prefix (updating parent as you create each segment) so that directory
creation cannot bypass the hardening; apply the same split-and-validate approach
to the other creation site referenced in the 103-122 range.
In `@crates/weaver-e2e/src/card_fixtures/python.rs`:
- Around line 25-26: The CardFixtureCase entries in
crates/weaver-e2e/src/card_fixtures/python.rs use Rust backslash-newline
continued string literals in their source fields; replace each such source value
with a concat!(...) invocation that joins the exact Python bytes (including \n
and indentation) so the resulting runtime string is identical, e.g., change
source: "line1\\\nline2\n" to source: concat!("line1\n", "line2\n"); update all
affected CardFixtureCase.source occurrences (the ones around the indicated
ranges) and keep the other metadata (name, file_name, line, column) unchanged,
ensuring the file remains rustfmt-clean and preserves behavior.
In `@crates/weaver-e2e/tests/support/fixture_io.rs`:
- Around line 14-24: Replace the match/panic and if-let/panic patterns with
.expect() calls: in open_fixture_dir call
Dir::open_ambient_dir(...).expect("open fixture temp dir") instead of matching
and panicking on Err, and in write_fixture call dir.write(file_name,
content).expect("write fixture") instead of using if let Err(...){ panic!(...)
}; reference the open_fixture_dir and write_fixture functions and the
Dir::open_ambient_dir and Dir::write calls to locate where to change.
In `@crates/weaver-e2e/tests/support/weaver_binary.rs`:
- Around line 58-64: Replace the map_or_else + panic pattern in function
workspace_root with an expect-based assertion: after creating manifest_dir (from
env!("CARGO_MANIFEST_DIR")), call
manifest_dir.parent().and_then(Path::parent).expect("workspace root should exist
for e2e tests").to_path_buf() so the code uses .expect() instead of
map_or_else(|| panic!(...)).
In `@crates/weaver-graph/src/tests/behaviour.rs`:
- Around line 15-16: The file imports and uses the forbidden macro
allow_fixture_expansion_lints; remove the import use
weaver_test_macros::allow_fixture_expansion_lints and replace its blanket
suppression by adding a scoped attribute on the fixture declaration (e.g.
annotate the world fixture with #[expect(unused_braces, reason = "fixture macro
expansion generates extra braces")] or similar) so only the unused_braces lint
is expected for that fixture; apply the same change to every other test fixture
that currently relies on allow_fixture_expansion_lints.
In `@crates/weaver-lsp-host/src/server.rs`:
- Around line 72-90: These accessor methods are pure field reads and must be
declared const to satisfy the missing_const_for_fn lint: change the signatures
of supports_definition, supports_references, supports_diagnostics,
supports_call_hierarchy, and supports_hover to const fn (keeping body and return
type the same) so each method becomes a compile-time constant accessor for the
respective fields (definition, references, diagnostics, call_hierarchy, hover).
In `@crates/weaver-plugin-rope/src/lib.rs`:
- Line 346: The code calls utf8_path.file_name().unwrap_or("file") which
silently falls back instead of propagating an error; update the logic in the
function that uses utf8_path (referencing utf8_path and the local variable
file_name) to handle the None case explicitly: either extend
validate_relative_path to reject paths lacking a file name or change the calling
code to match on utf8_path.file_name() and return a Result::Err (with a clear
error message) when it is None, rather than using unwrap_or; ensure the function
signature propagates the error (or converts to the crate's error type) so
callers receive the failure instead of a default "file" name.
In `@crates/weaver-plugin-rust-analyzer/src/tests/behaviour.rs`:
- Around line 12-13: Replace the macro-based suppression
allow_fixture_expansion_lints by removing its use/import and any invocations,
and instead add narrowly scoped #[expect(..., reason = "...")] attributes
directly on each affected test fixture (the fixture functions or modules
referenced around the original macro use and lines ~37-39) to suppress the
specific lint; update the attribute to name the exact lint being expected and
provide a clear reason string so it conforms to the guideline forbidding
#[allow].
In `@crates/weaver-plugins/src/capability/tests.rs`:
- Around line 206-210: The custom attribute macro allow_fixture_expansion_lints
currently expands to #[allow(unused_braces, reason = "...")]; change its
expansion to emit #[expect(unused_braces, reason = "...")] instead so test files
(e.g., the attribute used above the fixture fn contract) conform to the
guideline that only narrowly scoped #[expect(...)] is permitted; update the
macro definition (allow_fixture_expansion_lints) in the macro crate to replace
any emitted #[allow(...)] with #[expect(...)] while preserving the same lint and
reason text.
In `@crates/weaver-plugins/src/process.rs`:
- Around line 264-266: The timeout branch currently swallows results from
child.kill() and child.wait(), losing process-control errors; change those
drop(...) calls to explicitly handle their Results: call child.kill() and
match/handle Err to either log/attach the error or convert it into a distinct
PluginError (e.g., include a KillFailed or embed the kill error into
PluginError::Timeout), then call child.wait() and similarly handle Err (or
include its error in the returned PluginError::Timeout). Ensure you reference
the same child variable and return a proper Err(PluginError::Timeout { ... })
that contains the underlying kill/wait error information instead of discarding
it.
In `@crates/weaver-plugins/src/tests/capability_behaviour.rs`:
- Around line 55-57: Remove the forbidden #[allow_fixture_expansion_lints]
attribute from the fixture and instead apply a narrowly scoped
#[expect(unused_braces, reason = "fixture macro expansion triggers
unused_braces")] to the fixture function; in other words, update the fixture
declaration for fn world() -> CapabilityWorld { CapabilityWorld::default() } by
deleting the custom allow attribute and adding the indicated #[expect(...)]
attribute immediately above the fn so the lint suppression is scoped only to
this fixture expansion.
In `@crates/weaver-syntax/src/matcher/tests.rs`:
- Around line 6-7: Remove the global suppression import use
weaver_test_macros::allow_fixture_expansion_lints; and instead attach a narrowly
scoped #[expect(..., reason = "...")] attribute directly on each fixture
definition that previously required suppression (the fixtures referenced around
lines 12-14); replace the blanket allow with an explicit
#[expect(fixture_expansion_lint_name, reason = "brief reason for expecting this
lint")] placed immediately above each fixture function/fixture item (use the
actual lint name your tests trigger) so the suppression is localized to the
fixture only.
In `@crates/weaver-syntax/src/pattern.rs`:
- Around line 139-141: Rename the struct and API to use the Oxford "-ize"
spelling: change NormalisedSource to NormalizedSource (and the underlying
type/name String wrapper) and rename the function normalise_metavariables to
normalize_metavariables, then update every reference/usage, docs and comments
that mention NormalisedSource or normalise_metavariables to the new names to
keep identifiers and documentation consistent with en-GB-oxendict spelling.
In `@crates/weaver-syntax/src/rewriter.rs`:
- Around line 250-252: The function signature for process_metavar_ref uses
std::str::CharIndices without the required lifetime; change the parameter type
to include the elided lifetime (e.g., std::str::CharIndices<'_>) so it matches
the working signature used by count_dollars and compiles; update the function
declaration for process_metavar_ref accordingly wherever it's referenced.
In `@crates/weaver-syntax/src/tests/behaviour/mod.rs`:
- Line 70: Replace the forbidden #[allow_fixture_expansion_lints] attribute with
a narrowly scoped expect attribute: use #[expect(fixture_expansion_lints, reason
= "brief explanation why this test legitimately triggers fixture expansion
warnings")] applied to the same item (module/test) that currently has
#[allow_fixture_expansion_lints], remove any surrounding allow-style wrappers,
and ensure the reason string explains exactly why the warning is expected for
the specific test or module.
In `@crates/weaver-test-macros/src/lib.rs`:
- Around line 17-21: Replace the forbidden #[allow(...)] attribute on the macro
expansion with a narrowly scoped #[expect(...)] attribute: in the quote! block
where #[allow(unused_braces, reason = "fixture macro expansion triggers
unused-braces on expression bodies")] is used, change it to
#[expect(unused_braces, reason = "fixture macro expansion triggers unused-braces
on expression bodies")] so the same lint and reason remain but use the permitted
#[expect] form; update the attribute on the same item produced by quote! in
lib.rs accordingly.
In `@crates/weaverd/build.rs`:
- Around line 52-54: Detect when env::var_os("OUT_DIR") returns a non-UTF-8
OsString and emit a Cargo warning instead of silently dropping it: change the
out_dir_env construction so that if env::var_os("OUT_DIR") yields Some(os) but
os.to_str() is None you call println!("cargo:warning=...") (or eprintln!)
explaining OUT_DIR was non-UTF-8 and staging was skipped, then continue with
out_dir_for_target_profile(&target, &profile, None). Apply the same warning
logic to the other similar block (lines ~58-64) that parses OUT_DIR, and
reference the symbols out_dir_env, out_dir_for_target_profile, and
env::var_os("OUT_DIR") when making the change.
In `@crates/weaverd/src/dispatch/act/apply_patch/tests.rs`:
- Line 5: Remove the imported symbol allow_fixture_expansion_lints and any
module-level allow usage it enabled, and instead add a narrowly scoped
#[expect(..., reason = "...")] attribute directly on the specific test fixture
function in this file (replace the generic allow with an #[expect] that names
the fixture expansion lint and includes a brief reason); locate occurrences by
searching for the imported symbol allow_fixture_expansion_lints and the fixture
function defined in this file and apply the change there.
In `@crates/weaverd/src/dispatch/act/refactor/behaviour.rs`:
- Around line 105-107: The code extracts `language` via
language_name.ok_or_else(|| PluginError::NotFound { .. })? which is unreachable
because when requires_language && language_name.is_none() you early-return
refused_resolution; therefore remove this dead extraction and use the original
Option<&str> (`language_name`) consistently in the match arms (or alternatively
restructure the routing so `requires_language` is false only when
`language_name` is Some and then keep the extractor). Concretely, delete the
`let language = ...` line and update any uses of `language` to reference
`language_name`, ensuring match arms for UnsupportedLanguage and
ExplicitProviderMismatch operate on the Option rather than an extracted value;
remove the unused PluginError::NotFound variant from this path if it becomes
unreachable.
In `@crates/weaverd/src/dispatch/act/refactor/mod.rs`:
- Around line 239-244: Replace the unwrap_err() usage in
execute_plugin_and_handle_response by matching directly on the result of
params.runtime.execute(...) instead of unwrapping; on Ok(...) call
handle_successful_execution(response, writer, context) and on Err(error) call
write_execution_error(&error, params.selected_provider, args, writer)?; finally
return Ok(DispatchResult::with_status(1)) from the Err branch — this removes the
.unwrap_err() and follows the suggested match-based flow around
params.runtime.execute, write_execution_error, and handle_successful_execution.
In `@crates/weaverd/src/dispatch/act/refactor/request_building.rs`:
- Around line 20-32: The plugin request currently constructs payload/URI from
the raw CLI string (args.file) which can differ from the resolved path; update
the code that builds the FilePayload and any URI/rename inputs to use the
resolved file_path returned by resolve_file instead of args.file (e.g. replace
PathBuf::from(&args.file) or any workspace_root.join(args.file) usage with the
resolved file_path or its clone) so PluginRequest::with_arguments /
FilePayload::new and any rename/URI generation use the actual resolved path;
apply the same change in the other similar block referenced (lines 92-118) to
keep behavior consistent.
In `@crates/weaverd/src/dispatch/act/refactor/tests.rs`:
- Around line 6-7: Remove the global macro-based suppression
`allow_fixture_expansion_lints` and instead add a narrowly scoped attribute
`#[expect(<lint_name>, reason = "...")]` directly on the fixture declaration
(the test fixture item referenced in this file and also at locations mentioned
around lines 81–83); replace the macro import/use with no-op removal, pick the
exact lint that was being suppressed (e.g., the fixture-expansion lint name) and
attach the attribute to the fixture function/struct with a short justification
string so the suppression is scoped and auditable.
In `@crates/weaverd/src/dispatch/handler.rs`:
- Around line 141-147: The function finish_request_line currently uses an
if/else to return None for an empty Vec or Some(buffer) otherwise; replace the
body with a single idiomatic expression using Vec::is_empty().then_some(buffer)
(or buffer.is_empty().then_some(buffer) adjusted for ownership) so the function
becomes a one-liner while preserving behavior in finish_request_line.
In `@crates/weaverd/src/dispatch/observe/enrich_test_utils.rs`:
- Around line 83-86: The test helper run_enrichment_with_server is panicking on
semantic_backends_with_server failure while sibling run_non_ascii_enrichment
returns Result; make them consistent by changing run_enrichment_with_server to
propagate errors: update its signature to return Result<..., E> (matching
run_non_ascii_enrichment), replace the panic match around
semantic_backends_with_server with the ? operator, and propagate the error up to
callers (or update callers accordingly); reference symbols:
run_enrichment_with_server, run_non_ascii_enrichment,
semantic_backends_with_server.
In `@crates/weaverd/src/process/guard.rs`:
- Around line 69-72: The test-only call to record_health_event in guard.rs
currently swallows errors with let _ = record_health_event(...), which hides
mutex-poisoning or other failures; update the test block to handle the Result
returned by record_health_event (e.g., match or if let Err(e) = ...) and either
print a warning with eprintln! including the error and context (path,
snapshot.status) for best-effort logging, or propagate the error (return/unwrap)
if you want tests to fail; reference the record_health_event call in the
#[cfg(test)] block and adjust its error handling accordingly.
In `@crates/weaverd/src/process/guard/test_support.rs`:
- Around line 18-20: Add a Rustdoc comment (using ///) above the public function
health_events to describe its purpose, parameters and return value; mention that
it takes a Path reference representing the storage path and returns a
Vec<&'static str> of health event names (or similar brief description of the
values), and include any edge-case behavior (e.g., what happens on lock failure
via storage().lock()) so generated docs via cargo doc are complete and the API
intent is explicit.
In `@crates/weaverd/src/process/guard/tests.rs`:
- Around line 25-35: The call to
test_support::clear_health_events(paths.health_path()) in
setup_guard_with_health currently discards its Result, which breaks the
let_underscore_must_use lint; replace the discarded result with a checked call
using .expect("clear health events should succeed") so the helper mirrors the
other .expect() uses (locate the setup_guard_with_health function and update the
test_support::clear_health_events invocation).
In `@Makefile`:
- Around line 32-35: The Makefile lint target uses cargo doc without the
--workspace flag, so update the lint recipe (target name: lint) to invoke cargo
doc --no-deps --workspace (i.e., add the --workspace flag to the existing
RUSTDOCFLAGS/"$(RUSTDOC_FLAGS)" $(CARGO) doc --no-deps line) so documentation is
generated for all workspace members rather than only the root crate.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 95f187c8-abab-4b1f-aa47-3619d7254265
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (299)
.cargo/config.toml.github/workflows/build-and-package.yml.github/workflows/ci.yml.github/workflows/release.yml.gitignore.rustfmt.tomlCargo.tomlMakefilecrates/sempai-core/Cargo.tomlcrates/sempai-core/src/capture.rscrates/sempai-core/src/config.rscrates/sempai-core/src/diagnostic.rscrates/sempai-core/src/language.rscrates/sempai-core/src/match_result.rscrates/sempai-core/src/span.rscrates/sempai-core/src/test_support.rscrates/sempai-core/src/tests/behaviour.rscrates/sempai-core/src/tests/capture_tests.rscrates/sempai-core/src/tests/match_tests.rscrates/sempai-yaml/Cargo.tomlcrates/sempai-yaml/src/lib.rscrates/sempai-yaml/src/model.rscrates/sempai-yaml/src/parser/builders.rscrates/sempai-yaml/src/parser/mod.rscrates/sempai-yaml/src/project_depends_on.rscrates/sempai-yaml/src/raw.rscrates/sempai-yaml/src/source_map.rscrates/sempai-yaml/src/tests/behaviour.rscrates/sempai-yaml/src/tests/parser_tests/mod.rscrates/sempai-yaml/src/tests/taint_tests.rscrates/sempai-yaml/src/tests/test_helpers.rscrates/sempai/Cargo.tomlcrates/sempai/src/engine.rscrates/sempai/src/lib.rscrates/sempai/src/tests/behaviour.rscrates/sempai/src/tests/engine_tests.rscrates/sempai/src/tests/reexport_tests.rscrates/weaver-build-util/Cargo.tomlcrates/weaver-build-util/src/lib.rscrates/weaver-cards/Cargo.tomlcrates/weaver-cards/src/cache.rscrates/weaver-cards/src/card.rscrates/weaver-cards/src/detail.rscrates/weaver-cards/src/extract/attachments.rscrates/weaver-cards/src/extract/candidates.rscrates/weaver-cards/src/extract/fingerprint.rscrates/weaver-cards/src/extract/languages/common.rscrates/weaver-cards/src/extract/languages/params.rscrates/weaver-cards/src/extract/languages/python.rscrates/weaver-cards/src/extract/languages/rust.rscrates/weaver-cards/src/extract/languages/structure.rscrates/weaver-cards/src/extract/languages/typescript.rscrates/weaver-cards/src/extract/mod.rscrates/weaver-cards/src/extract/positions.rscrates/weaver-cards/src/extract/state.rscrates/weaver-cards/src/extract/utils.rscrates/weaver-cards/src/graph_slice/budget.rscrates/weaver-cards/src/graph_slice/mod.rscrates/weaver-cards/src/graph_slice/parse.rscrates/weaver-cards/src/graph_slice/parse_helpers.rscrates/weaver-cards/src/graph_slice/parse_helpers_tests.rscrates/weaver-cards/src/graph_slice/request.rscrates/weaver-cards/src/graph_slice/request_tests.rscrates/weaver-cards/src/graph_slice/response.rscrates/weaver-cards/src/lib.rscrates/weaver-cards/src/request.rscrates/weaver-cards/src/symbol.rscrates/weaver-cards/src/tests/behaviour.rscrates/weaver-cards/src/tests/cache_behaviour.rscrates/weaver-cards/src/tests/cache_tests.rscrates/weaver-cards/src/tests/extractor/common.rscrates/weaver-cards/src/tests/extractor/error_cases_tests.rscrates/weaver-cards/src/tests/extractor/general_tests.rscrates/weaver-cards/src/tests/extractor/signature_decorator_tests.rscrates/weaver-cards/src/tests/extractor/symbol_kind_tests.rscrates/weaver-cards/src/tests/extractor/whitespace_stability_tests.rscrates/weaver-cards/src/tests/fixtures.rscrates/weaver-cards/src/tests/graph_slice_behaviour.rscrates/weaver-cards/src/tests/graph_slice_fixtures.rscrates/weaver-cards/src/tests/graph_slice_snapshot_tests.rscrates/weaver-cards/src/tests/round_trip_tests.rscrates/weaver-cards/src/tests/snapshot_tests.rscrates/weaver-cards/src/tests/test_utils.rscrates/weaver-cli/Cargo.tomlcrates/weaver-cli/build.rscrates/weaver-cli/src/actionable_guidance.rscrates/weaver-cli/src/config.rscrates/weaver-cli/src/daemon_output.rscrates/weaver-cli/src/discoverability.rscrates/weaver-cli/src/discoverability/fluent_entries.rscrates/weaver-cli/src/errors.rscrates/weaver-cli/src/lib.rscrates/weaver-cli/src/lifecycle/controller.rscrates/weaver-cli/src/lifecycle/error.rscrates/weaver-cli/src/lifecycle/monitoring.rscrates/weaver-cli/src/lifecycle/monitoring_tests.rscrates/weaver-cli/src/lifecycle/shutdown.rscrates/weaver-cli/src/lifecycle/shutdown_tests.rscrates/weaver-cli/src/lifecycle/socket.rscrates/weaver-cli/src/lifecycle/spawning.rscrates/weaver-cli/src/lifecycle/types.rscrates/weaver-cli/src/lifecycle/utils.rscrates/weaver-cli/src/localizer.rscrates/weaver-cli/src/main.rscrates/weaver-cli/src/output/mod.rscrates/weaver-cli/src/output/models.rscrates/weaver-cli/src/output/render.rscrates/weaver-cli/src/output/source.rscrates/weaver-cli/src/runtime_utils.rscrates/weaver-cli/src/tests/behaviour.rscrates/weaver-cli/src/tests/support/fake_daemon.rscrates/weaver-cli/src/tests/support/lifecycle.rscrates/weaver-cli/src/tests/support/mod.rscrates/weaver-cli/src/tests/unit.rscrates/weaver-cli/src/tests/unit/actionable_guidance.rscrates/weaver-cli/src/tests/unit/after_help.rscrates/weaver-cli/src/tests/unit/auto_start.rscrates/weaver-cli/src/tests/unit/bare_invocation.rscrates/weaver-cli/src/tests/unit/discoverability.rscrates/weaver-cli/src/tests/unit/missing_operation_guidance.rscrates/weaver-cli/src/tests/unit/version_output.rscrates/weaver-cli/src/transport.rscrates/weaver-cli/tests/main_entry.rscrates/weaver-config/Cargo.tomlcrates/weaver-config/src/capability.rscrates/weaver-config/src/defaults.rscrates/weaver-config/src/lib.rscrates/weaver-config/src/runtime.rscrates/weaver-config/src/socket/mod.rscrates/weaver-config/src/socket/preparation.rscrates/weaver-config/tests/configuration_failfast.rscrates/weaver-config/tests/configuration_precedence.rscrates/weaver-e2e/Cargo.tomlcrates/weaver-e2e/src/card_fixtures/python.rscrates/weaver-e2e/src/card_fixtures/rust.rscrates/weaver-e2e/src/lib.rscrates/weaver-e2e/src/lsp_client.rscrates/weaver-e2e/src/pyrefly.rscrates/weaver-e2e/tests/call_hierarchy.rscrates/weaver-e2e/tests/definition_snapshots.rscrates/weaver-e2e/tests/get_card_snapshots.rscrates/weaver-e2e/tests/grep_snapshots.rscrates/weaver-e2e/tests/human_output.rscrates/weaver-e2e/tests/refactor_rope_cli_snapshots.rscrates/weaver-e2e/tests/refactor_rust_analyzer_cli_snapshots.rscrates/weaver-e2e/tests/rewrite_snapshots.rscrates/weaver-e2e/tests/support/fixture_io.rscrates/weaver-e2e/tests/support/weaver_binary.rscrates/weaver-e2e/tests/test_support/mod.rscrates/weaver-e2e/tests/weaver_graph.rscrates/weaver-graph/Cargo.tomlcrates/weaver-graph/src/edge.rscrates/weaver-graph/src/error.rscrates/weaver-graph/src/graph.rscrates/weaver-graph/src/lib.rscrates/weaver-graph/src/node.rscrates/weaver-graph/src/provider.rscrates/weaver-graph/src/tests/behaviour.rscrates/weaver-graph/src/tests/mod.rscrates/weaver-graph/src/tests/provider.rscrates/weaver-graph/src/tests/support.rscrates/weaver-graph/src/uri.rscrates/weaver-lsp-host/Cargo.tomlcrates/weaver-lsp-host/src/adapter/config.rscrates/weaver-lsp-host/src/adapter/error.rscrates/weaver-lsp-host/src/adapter/jsonrpc.rscrates/weaver-lsp-host/src/adapter/lifecycle.rscrates/weaver-lsp-host/src/adapter/messaging.rscrates/weaver-lsp-host/src/adapter/process.rscrates/weaver-lsp-host/src/adapter/trait_impl.rscrates/weaver-lsp-host/src/adapter/transport.rscrates/weaver-lsp-host/src/capability.rscrates/weaver-lsp-host/src/doc_support.rscrates/weaver-lsp-host/src/errors.rscrates/weaver-lsp-host/src/host.rscrates/weaver-lsp-host/src/language.rscrates/weaver-lsp-host/src/server.rscrates/weaver-lsp-host/src/tests/adapter_behaviour.rscrates/weaver-lsp-host/src/tests/behaviour.rscrates/weaver-lsp-host/src/tests/support/mod.rscrates/weaver-lsp-host/src/tests/support/recording_server.rscrates/weaver-lsp-host/src/tests/support/world.rscrates/weaver-lsp-host/src/tests/unit.rscrates/weaver-plugin-rope/Cargo.tomlcrates/weaver-plugin-rope/src/arguments.rscrates/weaver-plugin-rope/src/lib.rscrates/weaver-plugin-rope/src/tests/behaviour.rscrates/weaver-plugin-rope/src/tests/mod.rscrates/weaver-plugin-rust-analyzer/Cargo.tomlcrates/weaver-plugin-rust-analyzer/src/arguments.rscrates/weaver-plugin-rust-analyzer/src/failure.rscrates/weaver-plugin-rust-analyzer/src/lib.rscrates/weaver-plugin-rust-analyzer/src/lsp/jsonrpc.rscrates/weaver-plugin-rust-analyzer/src/lsp/mod.rscrates/weaver-plugin-rust-analyzer/src/lsp/text_edits.rscrates/weaver-plugin-rust-analyzer/src/tests/argument_validation.rscrates/weaver-plugin-rust-analyzer/src/tests/behaviour.rscrates/weaver-plugin-rust-analyzer/src/tests/dispatch_layer.rscrates/weaver-plugin-rust-analyzer/src/tests/mod.rscrates/weaver-plugin-rust-analyzer/src/tests/support.rscrates/weaver-plugins/Cargo.tomlcrates/weaver-plugins/src/capability/mod.rscrates/weaver-plugins/src/capability/rename_symbol.rscrates/weaver-plugins/src/capability/tests.rscrates/weaver-plugins/src/error/mod.rscrates/weaver-plugins/src/error/tests.rscrates/weaver-plugins/src/lib.rscrates/weaver-plugins/src/manifest/mod.rscrates/weaver-plugins/src/manifest/tests.rscrates/weaver-plugins/src/process.rscrates/weaver-plugins/src/protocol/mod.rscrates/weaver-plugins/src/protocol/tests.rscrates/weaver-plugins/src/registry/mod.rscrates/weaver-plugins/src/registry/tests.rscrates/weaver-plugins/src/runner/mod.rscrates/weaver-plugins/src/runner/tests.rscrates/weaver-plugins/src/tests/behaviour.rscrates/weaver-plugins/src/tests/capability_behaviour.rscrates/weaver-plugins/src/tests/mod.rscrates/weaver-sandbox/src/env_guard.rscrates/weaver-sandbox/src/error.rscrates/weaver-sandbox/src/lib.rscrates/weaver-sandbox/src/profile.rscrates/weaver-sandbox/src/runtime.rscrates/weaver-sandbox/src/sandbox.rscrates/weaver-syntax/Cargo.tomlcrates/weaver-syntax/src/language.rscrates/weaver-syntax/src/lib.rscrates/weaver-syntax/src/matcher/capture.rscrates/weaver-syntax/src/matcher/matching.rscrates/weaver-syntax/src/matcher/mod.rscrates/weaver-syntax/src/matcher/tests.rscrates/weaver-syntax/src/parser.rscrates/weaver-syntax/src/pattern.rscrates/weaver-syntax/src/rewriter.rscrates/weaver-syntax/src/syntactic_lock.rscrates/weaver-syntax/src/tests/behaviour/mod.rscrates/weaver-syntax/src/tests/behaviour/scenarios.rscrates/weaver-test-macros/Cargo.tomlcrates/weaver-test-macros/src/lib.rscrates/weaverd/Cargo.tomlcrates/weaverd/build.rscrates/weaverd/src/backends.rscrates/weaverd/src/bootstrap.rscrates/weaverd/src/dispatch/act/apply_patch/errors.rscrates/weaverd/src/dispatch/act/apply_patch/matcher.rscrates/weaverd/src/dispatch/act/apply_patch/mod.rscrates/weaverd/src/dispatch/act/apply_patch/parser.rscrates/weaverd/src/dispatch/act/apply_patch/parser/state.rscrates/weaverd/src/dispatch/act/apply_patch/semantic_lock.rscrates/weaverd/src/dispatch/act/apply_patch/tests.rscrates/weaverd/src/dispatch/act/apply_patch/types.rscrates/weaverd/src/dispatch/act/refactor/arguments.rscrates/weaverd/src/dispatch/act/refactor/behaviour.rscrates/weaverd/src/dispatch/act/refactor/candidates.rscrates/weaverd/src/dispatch/act/refactor/contract_tests.rscrates/weaverd/src/dispatch/act/refactor/manifests.rscrates/weaverd/src/dispatch/act/refactor/mod.rscrates/weaverd/src/dispatch/act/refactor/plugin_paths.rscrates/weaverd/src/dispatch/act/refactor/refactor_helpers.rscrates/weaverd/src/dispatch/act/refactor/refusal.rscrates/weaverd/src/dispatch/act/refactor/request_building.rscrates/weaverd/src/dispatch/act/refactor/resolution.rscrates/weaverd/src/dispatch/act/refactor/resolution_tests.rscrates/weaverd/src/dispatch/act/refactor/response_handling.rscrates/weaverd/src/dispatch/act/refactor/tests.rscrates/weaverd/src/dispatch/backend_manager.rscrates/weaverd/src/dispatch/errors.rscrates/weaverd/src/dispatch/handler.rscrates/weaverd/src/dispatch/observe/arguments.rscrates/weaverd/src/dispatch/observe/enrich.rscrates/weaverd/src/dispatch/observe/enrich_test_utils.rscrates/weaverd/src/dispatch/observe/enrich_tests.rscrates/weaverd/src/dispatch/observe/get_card.rscrates/weaverd/src/dispatch/observe/get_card_tests.rscrates/weaverd/src/dispatch/observe/get_definition.rscrates/weaverd/src/dispatch/observe/graph_slice.rscrates/weaverd/src/dispatch/observe/graph_slice_tests.rscrates/weaverd/src/dispatch/observe/responses.rscrates/weaverd/src/dispatch/observe/test_support.rscrates/weaverd/src/dispatch/request.rscrates/weaverd/src/dispatch/response.rscrates/weaverd/src/dispatch/router.rscrates/weaverd/src/dispatch/router/tests.rscrates/weaverd/src/health.rscrates/weaverd/src/lib.rscrates/weaverd/src/process/daemonizer.rscrates/weaverd/src/process/errors.rscrates/weaverd/src/process/files.rscrates/weaverd/src/process/guard.rscrates/weaverd/src/process/guard/test_support.rscrates/weaverd/src/process/guard/tests.rscrates/weaverd/src/process/launch.rscrates/weaverd/src/process/mod.rscrates/weaverd/src/process/shutdown.rscrates/weaverd/src/safety_harness/edit.rscrates/weaverd/src/safety_harness/error.rscrates/weaverd/src/safety_harness/locks.rscrates/weaverd/src/safety_harness/mod.rs
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/weaver-plugin-rust-analyzer/src/lib.rs`:
- Around line 282-291: The code currently uses
utf8_path.file_name().unwrap_or("file") which silently falls back to "file" for
directory-like inputs; change this to explicitly handle the None case by
returning a RustAnalyzerAdapterError::InvalidPath (include the resolved
absolute_path and a brief message) instead of writing to "file". Locate the
block around resolve_workspace_path and open_workspace_target_dir and replace
the unwrap_or on file_name() with an if-let or ok_or_else that returns
Err(RustAnalyzerAdapterError::InvalidPath { path: absolute_path.clone(), /*
message */ }) when file_name() is None before calling target_dir.write().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c7474937-933b-46bf-8cc8-15c77c13b61d
📒 Files selected for processing (4)
crates/weaver-plugin-rope/src/lib.rscrates/weaver-plugin-rope/src/tests/mod.rscrates/weaver-plugin-rust-analyzer/src/lib.rscrates/weaver-plugin-rust-analyzer/src/tests/mod.rs
Remove the redundant initial insertion of the reserved refactoring\nkey from plugin argument construction. The effective operation is\nalready written back later, so the earlier value only created\nconfusing overwrite behaviour.\n\nAlso consume the trimmed owned key directly when storing extra\narguments and collapse the splitn branch to the two cases that can\nactually occur. This keeps the request payload logic tighter\nwithout changing behaviour.
Extract a shared assertion helper for invalid URI cases in the\nrust-analyzer path utility tests and iterate the affected inputs in\nsmall table-driven loops.\n\nThe production path handling stays unchanged. The updated tests keep\ncurrent behaviour intact, including the existing empty-path error for\nroot-only file URIs.
Guard the rename-symbol request mapping against callers that pass\nboth 'offset' and 'position' in the extra argument set. The old\nlogic silently overwrote 'position' when offset remapping ran.\n\nReturn an invalid-arguments error instead and cover the behaviour\nwith a contract test so the rename request payload keeps a single\nsource of truth for the target position.
Update the CI workflow to install Nixie from the published\nPyPI package instead of relying on a GitHub repository install path.\n\nAlso run the Mermaid validation gate in CI so diagram checks use the\nsame tool source as local development.
- Reject ParentDir/RootDir/Prefix with InvalidPath (include component + path in message). - Skip only CurDir; keep Normal with UTF-8 validation. - Add unit tests to exercise the new error paths.
Install `nixie` in CI from the pinned `nixie-cli==1.0.0` PyPI package instead of relying on an unpinned package reference. Keep the workflow aligned with the requested distribution source and make the tool version explicit for reproducible CI runs.
Reconcile the rebased branch with `origin/main` by restoring the test and helper changes needed for the Whitaker and Clippy gates to pass again. - update the refactor test helpers so `weaverd` stays under the file-size limit and rollback helpers propagate errors instead of panicking - keep the plugin and e2e fixture adjustments that narrow lint suppressions and preserve clearer failure handling after the merge - add `codegen-backends` to `rust-toolchain.toml` so the toolchain component list matches the requested post-rebase configuration
Extract small stderr stream helpers in the refactor routing test support so the provider mismatch and automatic resolution paths reuse the same JSON stream writer. Preserve the existing stderr payload shape and boolean return semantics while removing the duplicated `write_json_line` construction.
Apply the requested review fixes across CI, documentation, socket endpoint handling, and test support code. - Update workflow and release metadata so the release job waits on the FreeBSD build and the repo ignores the intended editor artefacts. - Harden Unix socket endpoint parsing and round-tripping, and improve absolute workspace path reporting in capability-based file writes. - Replace panic-prone helper `expect` patterns in non-`#[test]` code with explicit handling so Whitaker passes without weakening the lint scope. - Split the `observe get-card` semantic tests into a sibling module to keep the test module below the line-count limit.
Tighten the verified lifecycle and path-handling fixes from the latest review pass. - Pass precomputed runtime probe data into degraded-status reporting so the controller avoids re-probing state. - Return an explicit timeout error from the fake daemon accept loop. - Decode Unix socket paths with `percent_encoding` and add regression coverage for space and percent-encoded inputs. - Reuse `write_workspace_file` for rust-analyzer stub manifests and document the path utility helpers more clearly. - Replace panic-based test setup in the apply-patch fixture and matcher helpers with small `Result` or shared panic wrappers where appropriate. Keep the remaining `refactor_helpers` synthetic references in place, because removing them still reintroduces `dead_code` failures under the current `#[path]`-included test-module layout.
Delay degraded-status probing until after the healthy snapshot fast path so transient socket or PID probe failures do not mask a healthy daemon. Reject malformed TCP socket URLs that include userinfo, extra path segments, queries, or fragments, and cover those cases with unit tests. Propagate fixture write failures in the call-hierarchy test setup and clean up the rust-analyzer text-edit helpers by clarifying names, removing the tuple-based UTF-8 offset API, and aligning the Windows prefix test with the intended validation branch.
…xpect(dead_code,…)] on cross-included helpers Remove the synthetic reference tests from `refactor_helpers.rs` and replace them with item-level dead-code expectations on the shared test helpers that are cross-included via `#[path]`. Keep the expectations narrow, retain a per-item justification, and add a matching item-level allowance for `unfulfilled_lint_expectations` where mixed compilation units make the helper live in some includes and dead in others.
Remove the backslash-escaped newline from the daemon status message so `concat!()` combines plain adjacent string literals directly. Update the rust-analyzer path utility rustdoc to use the repository's preferred Oxford spelling for "Normalize".
Preserve runtime path derivation errors instead of collapsing every failure to a not-running daemon state. Keep only the missing socket parent case as the absent-runtime path. Convert invalid URI test assertions to return `Result` and remove broad lint suppressions from refactor helper test support by making the cross-included helpers explicitly referenced in each module.
Propagate runtime directory probe errors instead of treating every filesystem failure as a missing daemon runtime directory. Reject empty and dot-only paths in `path_to_slash` so direct callers get the same invalid-path behaviour expected by request path validation.
Propagate readonly runtime path derivation failures instead of treating a missing socket parent as a not-running daemon. This keeps invalid socket configuration visible to lifecycle callers. Reject paths that normalize to no slash components in `path_to_slash`, so inputs such as `./` and `././` fail instead of returning an empty path.
Return `RuntimePaths` directly from `check_daemon_paths` when configuration-derived paths are available, and remove the unreachable `None` status branch.
Use the pattern stem to decide whether the Makefile build rule should pass `--release`, so target names containing `release` do not change the build mode accidentally. Simplify daemon runtime path error mapping and keep the socket status message split across coherent string fragments.
Expose `NIXIE_FLAGS` so CI can keep `--no-sandbox` by default while local runs can clear or replace the flag. Keep the daemon status command text contiguous in the socket-without-PID message.
Install `whitaker-installer` from the pinned Whitaker fix revision and run it with `--cranelift`. Remove the separate rustup cranelift component step so CI exercises the installer path being tested.
Removed 'codegen-backends' from the toolchain components.
Add the pinned Astral setup-uv action before the Nixie install step and use `uv tool install` instead of invoking pip directly.
Pin the release binary build steps to the shared-actions commit that removes the `+<toolchain>` override from `cross build` invocations.
lodyai
Bot
force-pushed
the
adopt-whitaker-lints
branch
from
aab57af to
5702998
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Stop forcing `clang` and `mold` for Linux targets from the workspace Cargo config. The release packaging action builds under `cross`, and the x86_64 cross image does not provide `clang`, so build scripts fail before Weaver compiles. Keep Cranelift scoped to development builds and update the README prerequisites to match.
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.
No description provided.