Skip to content

SOW-0172: Rust log writer accepts relative directories - #23

Open
ktsaou wants to merge 7 commits into
sow-0153/docs-clarityfrom
sow-0172/rust-log-relative-dir
Open

ktsaou wants to merge 7 commits into
sow-0153/docs-clarityfrom
sow-0172/rust-log-relative-dir

Conversation

@ktsaou

@ktsaou ktsaou commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Stacked on #22. Completes SOW-0172 (.agents/sow/done/SOW-0172-20261103-rust-log-relative-directory.md).

Decisions

  • User decision: option A. Rust Log makes the directory absolute once, at construction, without following symlinks.

Problem

  • Rust Log::new(".local/x", ...) created <machine-id>/ and then failed on the first write with failed to create journal file. journal-registry File::from_path rejects relative paths, and create_chain discarded its canonicalize() result.
  • In root-retention mode, pruning skipped every file under a relative root without any error.
  • An empty directory created <machine-id>/ in the working directory before failing. Go refuses an empty directory up front.

Changes

  • Log::new_inner resolves the directory with std::path::absolute and returns InvalidPath on failure, which includes the empty path.
  • Rust path accessors (configured_directory, journal_directory, active_path) now return absolute paths. Go keeps the path as given. This is documented in docs/Writer-APIs.md, product-scope.md and the rustdoc.
  • Tests:
    • Rust: relative writer (write, rotate, reopen), relative root pruning, and empty-path refusal. All three fail without the fix.
    • Go: matching relative-directory and empty-directory tests.
  • New pending SOW-0173: Rust Registry::watch_directory finds no files under a relative directory. It needs a decision.

Validation

  • go vet ./journal and go test ./... pass.
  • cargo test --workspace passes.
  • Clippy reports nothing in the changed files. Older lints elsewhere predate this change.
  • check_wiki_docs.py: OK. SOW audit: clean.

Summary by cubic

Makes the Rust Log writer accept relative directories like Go does: the directory is resolved to absolute once at construction, so writes, rotation, reopening, and root-retention pruning work, and an empty directory is refused upfront with InvalidPath instead of creating a stray <machine-id>/ directory before failing on the first write. Rust path accessors now return absolute paths while Go returns the path as given. The registry's related gap is tracked as pending SOW-0173: Registry::watch_directory silently finds no files under a relative directory.

Written for commit 17df96d. Summary will update on new commits.

View guided diff Turn on auto-fix

Rust Log resolves a relative directory once at construction, so default
and root-retention modes write, rotate and prune as Go does, and an empty
directory is refused before anything is created. Track the registry's
same-shape gap as SOW-0173.
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 76b2e1d0-87e9-4a07-bfa4-509cedf5dfdc

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@codacy-production

Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 23 complexity · 2 duplication

Metric Results
Complexity 23
Duplication 2

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

@codacy-production codacy-production Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

⚠️ Resolve issues before merging

The Rust constructor can accept an empty path as the current directory instead of rejecting it, potentially creating files in an unintended location. This should be fixed before merging.

The absolute-path regression scenario is not covered by the reported tests. No uncovered complex files were identified.

Test suggestions

  • Rust relative directory writes entries, rotates, and reopens successfully.
  • Rust root-retention mode prunes files under a relative directory.
  • Rust rejects an empty directory at construction with InvalidPath and creates nothing.
  • Rust path accessors report absolute paths for a relative input.
  • Go relative-directory writing, rotation, and reopening regression test.
  • Go rejects an empty directory.
  • Absolute-path behavior remains unchanged.
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Absolute-path behavior remains unchanged.

Make this review better

TIP Add a ticket key to the branch or title so the reviewer can check the PR against it. Or tell us which ticket tracker you use here.

TIP How was this review? Give us feedback

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

All reported issues were addressed across 9 files

Architecture diagram
sequenceDiagram
    participant Caller as Rust Caller
    participant Log as Rust Log::new_inner
    participant path as std::path::absolute
    participant Chain as Chain (journal-log-writer)
    participant Reg as journal-registry (File::from_path)
    participant FS as Filesystem
    participant Root as Root Retention (prune_root)

    Note over Caller,Root: Rust Log with Relative Directory (Option A - Resolved at Construction)

    Caller->>Log: Log::new(".local/x", config)
    Log->>path: NEW: std::path::absolute(".local/x")
    alt Absolute path resolution succeeds
        path-->>Log: /abs/path/.local/x
        Log->>Log: Store absolute path
        Log->>Chain: Initialize with absolute path
        Chain->>Reg: File::from_path(abs_path)
        Reg-->>Chain: Valid file handle
        Chain->>FS: NEW: create journal files
        Log-->>Caller: Ok(Log)

        Note over Caller,FS: Writes and Rotation Now Work
        Caller->>Log: write_entry() / rotate()
        Log->>FS: Write to absolute path
        FS-->>Log: Success
        Log-->>Caller: Ok

        Note over Caller,FS: Root Retention Mode Works
        Caller->>Root: set_root_retention_policy() + maintain()
        Root->>Reg: Scan absolute root
        Reg-->>Root: Files found
        Root->>FS: CHANGED: prune files (oldest removed)
        FS-->>Root: Deleted
    else Absolute path resolution fails (empty path)
        path-->>Log: Error
        Log-->>Caller: NEW: Err(InvalidPath)
        Note over Caller,Log: Empty directory refused upfront (matches Go)
    end

    Note over Caller,Log: Path Accessors Return Absolute Paths (NEW)
    Caller->>Log: configured_directory()
    Log-->>Caller: Absolute path
    Caller->>Log: journal_directory() / active_path()
    Log-->>Caller: Absolute paths

    Note over Reg,FS: FOLLOW-UP GAP (SOW-0173, not in this PR)
    Caller->>Reg: watch_directory(".local/y") 
    Reg->>FS: Scan yields relative paths
    Reg->>Reg: File::from_path(relative) returns None
    Reg-->>Caller: Ok (0 files) - SILENT FAILURE
Loading

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

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread go/journal/log_test.go Resolved
Comment thread docs/Writer-APIs.md Resolved
Empty-directory tests in both SDKs now assert nothing was created. Relative
path tests are Unix-only because Windows temp directories can sit on another
drive. Docs and SOW records describe Rust's lexical resolution accurately.
# Conflicts:
#	.agents/sow/SOW-status.md
#	.agents/sow/pending/SOW-0173-20261104-rust-registry-relative-directory.md

This branch has not been deployed

No deployments
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.

1 participant