Conversation
PR SummaryMedium Risk Overview
Written by Cursor Bugbot for commit 8d0b220. This will update automatically on new commits. Configure here. |
|
Caution Review failedThe pull request is closed. WalkthroughAdds a dirs subsystem to trogon-std: six public directory traits, a cross-platform SystemDirs implementation resolving OS-specific paths, a FixedDirs test-support implementation, module wiring with re-exports, and small doc tweaks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
rsworkspace/crates/trogon-std/src/dirs/mod.rs (1)
15-23: Nit: theignored doc example will silently rot.The FixedDirs example (Lines 15-23) uses
```ignorewhich means it's never compiled or tested bycargo test --doc. Consider using```no_runwith thetest-supportfeature, or acfg_attr-based approach, so doc tests still catch breakage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/mod.rs` around lines 15 - 23, The doc example for FixedDirs is marked with ```ignore so it isn't compiled; update it so doc-tests run without failing by changing the fence to ```no_run and gating the example on the test-support feature (or use cfg_attr to only compile the example when that feature is enabled), and reference the same symbols (FixedDirs, DirKind, StateDir, get_log_dir) in the adjusted example; this ensures the snippet is exercised by cargo test --doc while still avoiding side effects when the feature isn't enabled.rsworkspace/crates/trogon-std/src/dirs/system.rs (3)
120-169: Tests rely on env vars (HOME/USERPROFILE) being set — fragile in minimal CI containers.These tests will fail in stripped-down environments where no home directory env var is configured. Consider adding a brief doc-comment noting this assumption, or guarding with
#[ignore]+ a CI-specific override if broader portability is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/system.rs` around lines 120 - 169, The unit tests in the tests module (module tests, functions like home_dir_returns_some, config_dir_returns_some, etc., and the generic get_config_path helper) assume environment vars (HOME/USERPROFILE) exist and will fail in minimal CI; either add a short doc-comment on the tests explaining they require a configured home directory, or mark the fragile tests with #[ignore] (or conditionally ignore via cfg) so they don't run in stripped-down CI, or alternatively make the tests set required env vars at start (e.g., set HOME/USERPROFILE) before calling SystemDirs methods; update the tests module accordingly and reference the tests and SystemDirs when making the change.
94-105:data_local_dir_implis identical todata_dir_implon Linux — intentional?On Linux, both functions check
XDG_DATA_HOMEand fall back to~/.local/share. On Windows the distinction (APPDATA vs LOCALAPPDATA) is meaningful, but on Linux/macOS they return the same value. If this is intentional, a brief comment would clarify intent and prevent future "fix" attempts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/system.rs` around lines 94 - 105, The Linux/macOS behavior in data_local_dir_impl currently mirrors data_dir_impl (both use XDG_DATA_HOME or ~/.local/share), which looks suspicious; add a short clarifying comment inside data_local_dir_impl explaining that this parity is intentional (or document the reason for using the same fallback) so future readers won't refactor it accidentally—refer to data_local_dir_impl and data_dir_impl in the comment for clarity.
5-5: Consider deriving common traits onSystemDirs.As a public zero-sized unit struct, it would benefit from
Debug,Clone,Copyfor ergonomics and introspection.♻️ Proposed fix
+#[derive(Debug, Clone, Copy)] pub struct SystemDirs;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/system.rs` at line 5, SystemDirs is a public zero-sized unit struct that should derive common traits for ergonomics; update the declaration of SystemDirs to derive at least Debug, Clone, and Copy (and optionally PartialEq/Eq/Hash if desired) so callers can easily clone, copy and inspect instances; locate the pub struct SystemDirs; definition and add the #[derive(Debug, Clone, Copy)] (or include additional derives) to that struct.rsworkspace/crates/trogon-std/src/dirs/data.rs (1)
3-6:DataDirbundles two operations into a single trait, violating the one-trait-per-operation guideline.
data_diranddata_local_dirrepresent distinct directory concerns (e.g., roaming vs. local on Windows). Splitting intoDataDirandDataLocalDirwould be consistent with the rest of the module and the project guideline.♻️ Proposed split
data.rs
-pub trait DataDir { - fn data_dir(&self) -> Option<PathBuf>; - fn data_local_dir(&self) -> Option<PathBuf>; -} +pub trait DataDir { + fn data_dir(&self) -> Option<PathBuf>; +}data_local.rs (new file)
use std::path::PathBuf; pub trait DataLocalDir { fn data_local_dir(&self) -> Option<PathBuf>; }Then update
mod.rs,system.rs,fixed.rs, andlib.rsre-exports accordingly.As per coding guidelines:
rsworkspace/crates/**/*.rs: Prefer one trait per operation over a single trait with multiple operations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/data.rs` around lines 3 - 6, Split the combined trait DataDir into two single-operation traits: keep DataDir with only fn data_dir(&self) -> Option<PathBuf>, and add a new trait DataLocalDir with fn data_local_dir(&self) -> Option<PathBuf>; then update the concrete implementations (e.g., the types implementing DataDir/data_local_dir in system.rs and fixed.rs) to implement the appropriate trait(s) separately, and adjust module re-exports (mod and lib re-export lists) to export both DataDir and the new DataLocalDir so callers can use each trait independently.rsworkspace/crates/trogon-std/src/dirs/fixed.rs (2)
20-23: Add#[derive(Debug, Clone)]toFixedDirs.Without
Debug, assertion failures in tests produce no meaningful output. WithoutClone, consumers of thetest-supportfeature cannot fork a pre-configured instance across multiple test scenarios — a common pattern for fixture-sharing. BothHashMap<DirKind, PathBuf>and its contents already implement the required traits, so the derives cost nothing.♻️ Proposed fix
#[cfg(any(test, feature = "test-support"))] +#[derive(Debug, Clone)] pub struct FixedDirs { dirs: HashMap<DirKind, PathBuf>, }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/fixed.rs` around lines 20 - 23, Add missing derives to the FixedDirs struct: update the declaration of FixedDirs to include #[derive(Debug, Clone)] so test failures print meaningful output and callers can clone fixture instances; the inner fields (HashMap<DirKind, PathBuf>) already implement Debug and Clone so simply adding the derive attributes to the FixedDirs struct (reference: FixedDirs) fixes the issue.
72-81:DataDircombines two operations — consider splitting intoDataDir+DataLocalDir.Per the project coding guideline "prefer one trait per operation over a single trait with multiple operations",
data_diranddata_local_dirshould be separate traits. The violation originates inrsworkspace/crates/trogon-std/src/dirs/data.rs(lines 3–4), and is mirrored here in theFixedDirsimplementation. Splitting intoDataDirandDataLocalDirkeeps the trait surface consistent withHomeDir,ConfigDir,CacheDir, andStateDir.As per coding guidelines: "Prefer one trait per operation over a single trait with multiple operations."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/fixed.rs` around lines 72 - 81, Split the combined trait into two single-operation traits: keep/rename the existing DataDir trait to only expose data_dir(&self) and create a new DataLocalDir trait that exposes data_local_dir(&self); update the impl block for FixedDirs (the impl DataDir for FixedDirs that currently defines both data_dir and data_local_dir) into two impl blocks (impl DataDir for FixedDirs with data_dir and impl DataLocalDir for FixedDirs with data_local_dir) and adjust any references/tests that used the combined trait to use the appropriate single-operation trait names (ensure cfg attributes like #[cfg(any(test, feature = "test-support"))] remain on the new impl blocks).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rsworkspace/crates/trogon-std/src/dirs/system.rs`:
- Around line 47-118: The macOS implementation makes config_dir, data_dir,
data_local_dir and state_dir all resolve to ~/Library/Application Support which
can cause silent collisions; update state_dir_impl to return None on macOS (and
likewise return None on Windows to match the dirs crate behavior) instead of
aliasing Application Support, and add a brief comment in state_dir_impl
explaining that state_dir is intentionally None on macOS/Windows so callers must
create app-specific subpaths; keep home_dir_impl, config_dir_impl, data_dir_impl
and data_local_dir_impl unchanged.
---
Nitpick comments:
In `@rsworkspace/crates/trogon-std/src/dirs/data.rs`:
- Around line 3-6: Split the combined trait DataDir into two single-operation
traits: keep DataDir with only fn data_dir(&self) -> Option<PathBuf>, and add a
new trait DataLocalDir with fn data_local_dir(&self) -> Option<PathBuf>; then
update the concrete implementations (e.g., the types implementing
DataDir/data_local_dir in system.rs and fixed.rs) to implement the appropriate
trait(s) separately, and adjust module re-exports (mod and lib re-export lists)
to export both DataDir and the new DataLocalDir so callers can use each trait
independently.
In `@rsworkspace/crates/trogon-std/src/dirs/fixed.rs`:
- Around line 20-23: Add missing derives to the FixedDirs struct: update the
declaration of FixedDirs to include #[derive(Debug, Clone)] so test failures
print meaningful output and callers can clone fixture instances; the inner
fields (HashMap<DirKind, PathBuf>) already implement Debug and Clone so simply
adding the derive attributes to the FixedDirs struct (reference: FixedDirs)
fixes the issue.
- Around line 72-81: Split the combined trait into two single-operation traits:
keep/rename the existing DataDir trait to only expose data_dir(&self) and create
a new DataLocalDir trait that exposes data_local_dir(&self); update the impl
block for FixedDirs (the impl DataDir for FixedDirs that currently defines both
data_dir and data_local_dir) into two impl blocks (impl DataDir for FixedDirs
with data_dir and impl DataLocalDir for FixedDirs with data_local_dir) and
adjust any references/tests that used the combined trait to use the appropriate
single-operation trait names (ensure cfg attributes like #[cfg(any(test, feature
= "test-support"))] remain on the new impl blocks).
In `@rsworkspace/crates/trogon-std/src/dirs/mod.rs`:
- Around line 15-23: The doc example for FixedDirs is marked with ```ignore so
it isn't compiled; update it so doc-tests run without failing by changing the
fence to ```no_run and gating the example on the test-support feature (or use
cfg_attr to only compile the example when that feature is enabled), and
reference the same symbols (FixedDirs, DirKind, StateDir, get_log_dir) in the
adjusted example; this ensures the snippet is exercised by cargo test --doc
while still avoiding side effects when the feature isn't enabled.
In `@rsworkspace/crates/trogon-std/src/dirs/system.rs`:
- Around line 120-169: The unit tests in the tests module (module tests,
functions like home_dir_returns_some, config_dir_returns_some, etc., and the
generic get_config_path helper) assume environment vars (HOME/USERPROFILE) exist
and will fail in minimal CI; either add a short doc-comment on the tests
explaining they require a configured home directory, or mark the fragile tests
with #[ignore] (or conditionally ignore via cfg) so they don't run in
stripped-down CI, or alternatively make the tests set required env vars at start
(e.g., set HOME/USERPROFILE) before calling SystemDirs methods; update the tests
module accordingly and reference the tests and SystemDirs when making the
change.
- Around line 94-105: The Linux/macOS behavior in data_local_dir_impl currently
mirrors data_dir_impl (both use XDG_DATA_HOME or ~/.local/share), which looks
suspicious; add a short clarifying comment inside data_local_dir_impl explaining
that this parity is intentional (or document the reason for using the same
fallback) so future readers won't refactor it accidentally—refer to
data_local_dir_impl and data_dir_impl in the comment for clarity.
- Line 5: SystemDirs is a public zero-sized unit struct that should derive
common traits for ergonomics; update the declaration of SystemDirs to derive at
least Debug, Clone, and Copy (and optionally PartialEq/Eq/Hash if desired) so
callers can easily clone, copy and inspect instances; locate the pub struct
SystemDirs; definition and add the #[derive(Debug, Clone, Copy)] (or include
additional derives) to that struct.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
yordis
force-pushed
the
add-dirs
branch
3 times, most recently
from
0de50ef to
e9f610d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rsworkspace/crates/trogon-std/src/dirs/mod.rs`:
- Around line 5-13: The doctest imports ConfigDir but never uses it, causing
unused_imports warnings; update the example to remove the unused import or use
it—specifically edit the import line that currently reads use
trogon_std::dirs::{ConfigDir, StateDir, SystemDirs}; to only import the traits
actually used (e.g., StateDir and SystemDirs) or add a small use of ConfigDir in
the example (e.g., a helper function) so the compiler recognizes it; target the
doctest block surrounding the get_log_dir function and SystemDirs usage to apply
the change.
In `@rsworkspace/crates/trogon-std/src/lib.rs`:
- Line 7: The intra-doc links to FixedDirs (and the test doubles MemFs,
InMemoryEnv, MockClock) are dangling because those types are only re-exported
under #[cfg(any(test, feature = "test-support"))]; fix by either qualifying the
doc links (e.g. use the full path like crate::dirs::FixedDirs,
crate::dirs::MemFs, crate::dirs::InMemoryEnv, crate::dirs::MockClock in the
lib.rs module docs) or add a conditional re-export at the crate root to make
them available symmetrically (e.g. #[cfg(any(test, feature = "test-support"))]
pub use dirs::{FixedDirs, MemFs, InMemoryEnv, MockClock}), updating the
references to the unique symbols FixedDirs, MemFs, InMemoryEnv, and MockClock
accordingly.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rsworkspace/crates/trogon-std/src/dirs/fixed.rs (1)
33-36: Consider takingselfby value for a more ergonomic builder pattern.Currently
settakes&mut self, which requires callers to declarelet mut dirsbefore chaining. Takingselfby value and returningSelfwould allow one-liner construction:let dirs = FixedDirs::new() .set(DirKind::Home, "/home") .set(DirKind::Config, "/config");♻️ Proposed refactor
- pub fn set(&mut self, kind: DirKind, path: impl Into<PathBuf>) -> &mut Self { + pub fn set(mut self, kind: DirKind, path: impl Into<PathBuf>) -> Self { self.dirs.insert(kind, path.into()); self } - pub fn clear(&mut self) -> &mut Self { + pub fn clear(mut self) -> Self { self.dirs.clear(); self }This would require updating the test call sites to use
let dirs = ...instead oflet mut dirs, but the ergonomic win is worth it for a test-only builder.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rsworkspace/crates/trogon-std/src/dirs/fixed.rs` around lines 33 - 36, Change the builder-style method set to take ownership and return Self instead of &mut Self: update the signature of FixedDirs::set to accept self (e.g., pub fn set(mut self, kind: DirKind, path: impl Into<PathBuf>) -> Self), perform the insert on the owned instance and return it, and then update all call sites/tests to use non-mutable chained construction (e.g., let dirs = FixedDirs::new().set(...).set(...)) so they no longer require let mut.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rsworkspace/crates/trogon-std/src/dirs/fixed.rs`:
- Around line 33-36: Change the builder-style method set to take ownership and
return Self instead of &mut Self: update the signature of FixedDirs::set to
accept self (e.g., pub fn set(mut self, kind: DirKind, path: impl Into<PathBuf>)
-> Self), perform the insert on the owned instance and return it, and then
update all call sites/tests to use non-mutable chained construction (e.g., let
dirs = FixedDirs::new().set(...).set(...)) so they no longer require let mut.
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
jramirezhdez02
added a commit
that referenced
this pull request
Feb 27, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
jramirezhdez02
added a commit
that referenced
this pull request
Apr 7, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
mariorha
pushed a commit
that referenced
this pull request
Apr 7, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
jramirezhdez02
added a commit
that referenced
this pull request
Apr 9, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
mariorha
pushed a commit
that referenced
this pull request
Apr 9, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
mariorha
pushed a commit
that referenced
this pull request
Apr 10, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
jramirezhdez02
added a commit
that referenced
this pull request
Apr 13, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
mariorha
pushed a commit
that referenced
this pull request
Apr 13, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
mariorha
added a commit
that referenced
this pull request
Apr 14, 2026
…l-results TTL #3 — Pre-LLM heartbeat now reloads the KV revision and retries the claimed_at write on CAS conflict instead of just warning. Repeated conflicts no longer leave claimed_at stale, preventing startup recovery on another process from false-positively stealing an active run. If the reload reveals the promise reached a terminal state, the run stops cleanly to avoid duplicate side effects. #5 — System prompt pre-pin now reloads the revision and retries on CAS conflict instead of immediately reverting system_prompt to None. Reduces the window where a crash before the first checkpoint causes recovery to resume with a different system prompt. #7 — AGENT_TOOL_RESULTS bucket TTL raised to 2 × PROMISE_TTL (48 h). Tool results are written once and never updated, so their TTL counts from creation time. A run lasting up to 24 h could exhaust a 24 h TTL for tool results written early in the run; doubling the TTL ensures any result written at any point during a max-length run is still available at the very end.
mariorha
added a commit
that referenced
this pull request
Apr 14, 2026
…op_reason → Failed #7 — split max_tokens out of the catch-all stop_reason arm (keeps PermanentFailed) and change the true `other` arm to write `Failed` instead of PermanentFailed. A future Anthropic stop_reason that is transient would previously be discarded forever; now NATS redelivery can retry it up to the stream's delivery limit. #8 — add `recovery_count: u32` (#[serde(default)]) to AgentPromise. Each time startup recovery re-claims a stale Running promise the field is incremented. At MAX_RECOVERY_ATTEMPTS (5) the promise is marked PermanentFailed and the run is skipped, breaking the infinite cycle caused by deterministic crashes that prevent try_mark_permanent_failed_fresh from writing. Failed promises (NATS redelivery path) are not counted. Tests added: max_tokens_stop_reason_marks_promise_permanent_failed, unknown_stop_reason_marks_promise_failed_not_permanent, prepare_agent_over_recovery_limit_marks_permanent_failed_and_returns_none, prepare_agent_running_reclaim_increments_recovery_count.
mariorha
pushed a commit
that referenced
this pull request
Apr 15, 2026
#5 — heartbeat KV timeout now reloads revision and retries the write, matching the CAS-conflict arm. Under sustained NATS degradation the old code left claimed_at frozen; the reload+retry keeps it fresh and avoids false-positive stale detection by startup recovery. #2 — recovering resets to false when checkpointing is permanently disabled (payload exceeds all trim levels). Previously recovering stayed true forever, causing same-input tool calls in genuinely-new turns to return stale cached results (polling anti-pattern). New test recovering_resets_when_checkpointing_permanently_disabled covers the case. #7 — summarize_dropped_messages is now wrapped in a 30 s timeout. The call inherits a 5-minute LLM timeout which blocks the checkpoint write and leaves claimed_at stale for that window. 30 s is generous for a short summary; on timeout the empty Vec triggers the plain-trim fallback that was already there. #6 — list_running timeout raised from NATS_KV_TIMEOUT (10 s) to a dedicated LIST_RUNNING_TIMEOUT (2 min). The operation does N+1 KV reads; at 50+ stale promises under mild load the 10 s ceiling fires and silently skips recovery. 2 min covers hundreds of promises without blocking the consumer loop (recovery runs in a background task).
mariorha
pushed a commit
that referenced
this pull request
Apr 20, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
mariorha
pushed a commit
that referenced
this pull request
Apr 20, 2026
- backoff_shift_saturates_at_31_for_high_attempt_counts: verifies the
formula (attempts-1).min(31) saturates the shift count at 31 for all
attempt values >= 32, preventing u32 overflow in Duration arithmetic
- e2e_empty_query_string_appended_as_bare_question_mark: documents that
a URI with trailing '?' (empty query) causes uri.query() = Some(""),
which the proxy formats as "?" and appends to the upstream URL
Gaps #1 (double publish failure) and #8 (consumer stream error) are not
testable without introducing a trait abstraction over async_nats::Client.
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
dirsmodule totrogon-stdwith per-concern traits (HomeDir,ConfigDir,CacheDir,DataDir,StateDir) for platform directory resolutionSystemDirsprovides zero-cost production implementation using XDG/macOS/Windows conventionsFixedDirs(behindtest-supportfeature) provides aHashMap-backed test doubleTest plan
FixedDirs(default, set/get, clear, overwrite, chaining, generic usage)SystemDirs(all dirs returnSome, generic function usage)