Skip to content

SOW-0097: Single ownership of journal behavior in the SDK, used by both journalctl rewrites - #26

Open
ktsaou wants to merge 24 commits into
sow-0174/rust-registry-unwatchfrom
sow-0097/cli-sdk-single-owner
Open

ktsaou wants to merge 24 commits into
sow-0174/rust-registry-unwatchfrom
sow-0097/cli-sdk-single-owner

Conversation

@ktsaou

@ktsaou ktsaou commented Nov 8, 2026 •

Copy link
Copy Markdown
Member

Pending user review

Agent-chosen implementation choices (recorded in SOW-0097):

  • Export/JSON: new EntryEncoder (Go NewEntryEncoder + AppendExport/AppendJSON/AppendJSONPretty; Rust EntryEncoder::new + append_*) and the public printable rule IsPrintableText / is_printable_text. Follows systemd v262 output_export/output_json exactly; JSON keys are sorted (systemd's order is hash order).
  • Boot listing: kept the algorithm both CLIs already used and the query matrix validates, now read straight from the _BOOT_ID DATA entry arrays (no match flush, no position change). Only canonical lowercase _BOOT_ID values name a boot, as the old CLIs effectively did. New Reader.ListBoots (Go) and FileReader::list_boots (Rust).

Open decisions (tracked in SOW-0176; current behavior unchanged):

  • 8: legacy ExportEntry / export_entry_bytes keep the SOW-0043 lossless contract instead of becoming systemd-exact.
  • 9: Rust DirectoryReader::list_boots / SdJournal::list_boots keep the infallible Vec signature (header-summary fallback on read errors); Go returns an error.

Summary

SOW-0097: one owner per journal behavior. Behaviors the journalctl rewrites copied from the SDK now live in public SDK functions in both Go and Rust, and both CLIs call them:

  • directory file listing and path dedupe, header flags, export seqnum ID, unit-name SipHash (Go), journald field-name rule;
  • export/JSON rendering and the printable rule: both CLIs now match the v262 goldens in all six cases where they previously differed;
  • --list-boots: the SDK listed one boot per file; it now lists every boot. The SDK and both CLIs handle dirty archives and corrupted files the way v262 does.

Also: a production duplication gate in CI with a committed baseline (76 pairs / 5167 lines, down from 79 / 5299), and the single-ownership rule in AGENTS.md.

Performance

  • Export/JSON, 100k entries: Go 5.18 s -> 4.56 s (export), 7.93 s -> 6.38 s (json), 9.19 s -> 6.78 s (json-pretty); Rust 2.01 s -> 1.65 s, 2.66 s -> 2.09 s, 2.63 s -> 2.11 s. Output is byte-identical before and after.
  • --list-boots, 200 boots x 1000 entries: Go 33 -> 30 ms, Rust 10.1 -> 9.2 ms (v262 19.6 ms). 100k entries without _BOOT_ID DATA: Go 3.61 s -> 77 ms, Rust 1.50 s -> 46 ms.

Validation

  • go test ./... and cargo test --workspace pass.
  • journalctl query matrix vs the v262 oracle: 3586 cases pass, follow included.
  • Directory matrix (34 checks), mixed-directory matrix (42/42) and verify matrix (63 results) pass.
  • Docs: wiki check passes, and all 39 verified examples pass, including four new ones.
  • Old-vs-new --list-boots sweep over 1409 corpus files and 288 directories: no changes except 3 corrupted fixtures, which now print the v262 result.

Stack

Based on #25 (SOW-0174). Merge after it.


Summary by cubic

Makes the SDK the single owner of journal behavior in Go and Rust: both journalctl rewrites now call public SDK functions instead of carrying their own copies, and export/JSON rendering and boot listing now match systemd v262 exactly.

New EntryEncoder (Go NewEntryEncoder, Rust EntryEncoder::new) renders export, JSON, and JSON-pretty per v262; both CLIs match the v262 goldens in all six cases where they previously differed. The printable rule it uses is public and also drives short, cat, and verbose output.

ListBoots/list_boots reports every boot from _BOOT_ID DATA entry arrays instead of one per file header, and handles dirty archives and corrupted files like v262; Go refreshes before reading postings so long-lived readers see new entries.

  • Directory file listing and path dedupe, header flags, long unit-name SipHash, the journald field-name rule, version-4 UUIDs, and disposed-file microsecond naming are public SDK functions both CLIs call; the Rust Netdata layer classifies non-UTF-8 names as Go does.
  • Legacy ExportEntry/export_entry_bytes keep the lossless contract, now length-prefixing unsafe metadata and skipping fields with no values; open contract decisions are tracked in SOW-0176.
  • Export/JSON get faster (Go export 5.18s → 4.56s per 100k entries; boot listing without _BOOT_ID DATA drops from 3.61s to 77ms), and a CI duplication gate pins production clones at a committed baseline (76 pairs / 5167 lines, down from 79 / 5299).

Written for commit 5c15b9b. Summary will update on new commits.

View guided diff Turn on auto-fix

ktsaou added 17 commits November 7, 2026 22:33
…th languages

Both journalctl rewrites now render export, json and json-pretty through
the SDK EntryEncoder, which follows systemd v262 output_export and
output_json: metadata always printed, DATA in on-disk order, export stops
at an invalid field name, JSON skips it, 4096-byte null threshold unless
--all, and systemd's printable rule including Unicode noncharacters. The
printable rule is public (IsPrintableText / is_printable_text) and also
drives short, cat and verbose output. Goldens from the v262 oracle cover
the SDK and both CLIs. ExportEntry / export_entry_bytes keep fields with
invalid names, as before.
…th journalctl rewrites

ListBoots/list_boots reported one boot per file header. The SDK now reads
each boot's first and last entry from the _BOOT_ID DATA entry arrays, merges
them across files in directory order, and falls back to an entry-header scan
for files without _BOOT_ID DATA. Listing boots no longer changes the caller's
matches or position, follows reader tail recovery on dirty archives, and is
what both journalctl rewrites now call instead of their own copies.
…helpers

Adds verified Go and Rust wiki examples for EntryEncoder and boot listing,
lists the SDK helpers the journalctl rewrites now call, and records the
export/JSON and boot-listing contracts in the product spec.
Locks the reduced production duplication baseline (76 pairs, 5167 lines),
records validation, and tracks the open export and boot-listing contract
decisions, the verify-key item and the divergences found in SOW-0176.
@coderabbitai

coderabbitai Bot commented Nov 8, 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: 5548e3e0-2f95-4277-8a80-8aade8ddcc6c

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 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

Two blocking concerns remain: the Rust header-scan fallback can collapse multiple per-file boot records sharing an ID, and the duplication checker can execute an arbitrary caller-supplied command. Both should be addressed before merging.

The verification-key parity criterion remains unverified. The Go follow implementation is also highly complex and lacks dedicated coverage evidence, but this is a maintainability concern rather than a merge blocker.

About this PR

  • Public API parity is not complete: verification-key behavior remains unverified, and boot-listing error semantics differ between languages. These compatibility decisions should be explicitly resolved or gated before release.

Test suggestions

  • Systemd-compatible export, JSON, pretty JSON, field selection, binary values, repeated fields, invalid fields, and threshold handling in Go and Rust
  • Printable-text handling, including controls, invalid UTF-8, and Unicode noncharacters
  • Multi-boot listing from DATA indexes, header-scan fallback, canonical boot IDs, ordering, and index numbering
  • Boot listing preserves matches and reader position
  • Fresh-reader dirty-archive boot listing and recovery retry
  • Directory file discovery and path deduplication, including symlinks, machine-ID directories, archives, and ignored paths
  • Version-4 UUID bits and disposed-file microsecond timestamps
  • Non-UTF-8 Netdata journal filenames
  • Duplication checker classification, grouping, baseline comparison, and CI invocation
  • Verification-key grammar parity with systemd using an FSS-enabled oracle
Prompt proposal for missing tests
Consider implementing these tests if applicable:
1. Verification-key grammar parity with systemd using an FSS-enabled oracle

TIP How was this review? Give us feedback

Comment thread tests/duplication/check_duplication.py Resolved
Comment thread go/cmd/journalctl/main.go Resolved
Comment thread rust/src/journal/src/directory.rs Resolved
@codacy-production

codacy-production Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 278 complexity · -11 duplication

Metric Results
Complexity 278
Duplication -11

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.

@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.

7 issues found and verified against the latest diff

Confidence score: 3/5

  • file_boot_ranges in boots.rs can return a posting beyond the reader’s visible tail. Bound the search by the last visible entry, as the Go implementation does.
  • duplication.yml can let a PR bypass the duplication gate by changing the committed baseline alongside a new clone. Compare with the base revision’s baseline or reject unapproved baseline changes.
  • netdata.rs still routes files with non-UTF-8 namespace parents to LOCAL_SYSTEM. Handle the parent path without assuming it is UTF-8.
  • SOW-0176-20261108-single-ownership-followups.md is missing both the required implementation and review plan and the options, trade-offs, and recommendation needed for its decision gate. Add these before execution.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".agents/sow/pending/SOW-0176-20261108-single-ownership-followups.md">

<violation number="1" location=".agents/sow/pending/SOW-0176-20261108-single-ownership-followups.md:96">
P2: `SOW-0176` asks for decisions without presenting numbered options, trade-offs, or a recommendation. Add those alternatives and the supporting evidence so the pending SOW can satisfy the user-decision gate.</violation>

<violation number="2" location=".agents/sow/pending/SOW-0176-20261108-single-ownership-followups.md:107">
P2: Add the required `Implementation And Review Plan` before `## Execution Log`. Without it, this SOW has no recorded implementation owner, review gate, repository-boundary instructions, or failure-handling procedure for work that includes public-contract and data-retention decisions.</violation>
</file>

<file name="rust/README.md">

<violation number="1" location="rust/README.md:267">
P2: This fallback description is inaccurate: `DirectoryReader::list_boots` cannot report headers for files that directory opening discarded, and it merges equal header boot IDs. Describe the fallback as header metadata from successfully opened readers instead of promising one boot per unreadable file.</violation>
</file>

<file name="rust/src/journal/src/boots.rs">

<violation number="1" location="rust/src/journal/src/boots.rs:124">
P2: `file_boot_ranges` can report an entry beyond the reader's visible tail because these partition-point calls are not bounded by the last visible entry. Bound the posting search to the global entry-list tail, as the Go implementation does, so dirty or concurrently written journals cannot expose future entries in `--list-boots`.</violation>
</file>

<file name="rust/src/journal/src/netdata.rs">

<violation number="1" location="rust/src/journal/src/netdata.rs:3426">
P2: This byte-safe basename change still leaves non-UTF-8 namespace parents on the wrong source path. A file such as `machine.\xff/system.journal` now falls through to `LOCAL_SYSTEM` because `local_namespace_source_name` rejects the parent with `to_str()`, so `all-local-namespaces` and exact namespace selection disagree with Go; handle namespace parents using the same byte-safe representation.</violation>
</file>

<file name=".github/workflows/duplication.yml">

<violation number="1" location=".github/workflows/duplication.yml:34">
P2: This gate can be bypassed by changing the committed baseline in the same PR as a new clone. Compare against the base revision's baseline, or reject baseline changes unless they are separately approved, so the gate still detects growth.</violation>
</file>

<file name="AGENTS.md">

<violation number="1" location="AGENTS.md:72">
P2: This rule claims verification-key syntax already belongs to the SDK, but I7 remains an explicit unresolved exception and both CLIs still retain their parsers. Scope the rule or document that exception; otherwise it directs maintainers to remove oracle-specific handling before the behavior is verified.</violation>
</file>

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

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

Comment thread rust/src/journal/src/directory.rs Resolved
Comment thread .github/workflows/duplication.yml Resolved
Comment thread AGENTS.md Resolved
Comment thread tests/output/README.md Resolved
Comment thread go/journal/entry_output.go Resolved

@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.

1 issue found across 71 files

Confidence score: 3/5

  • boots.rs can list a boot that canonical _BOOT_ID matching would not find when the index contains only noncanonical values. Track field presence separately from canonical matches and gate the header scan on field presence.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="rust/src/journal/src/boots.rs">

<violation number="1" location="rust/src/journal/src/boots.rs:38">
P2: This condition treats an all-noncanonical `_BOOT_ID` index as absent, so the fallback scan lists a boot that canonical `_BOOT_ID` matching would not find. Track field presence independently and scan headers only when `_BOOT_ID` DATA is absent.</violation>
</file>
Architecture diagram
sequenceDiagram
    participant CLI as journalctl CLI (Go/Rust)
    participant FReader as FileReader
    participant DReader as DirectoryReader
    participant SDJ as SdJournal Facade
    participant Encoder as EntryEncoder
    participant Boots as Boot Listing Module
    participant DB as Journal DATA Store

    Note over CLI,Encoder: NEW: Single ownership of journal behavior in SDK

    CLI->>FReader: Open files/directory
    FReader->>DB: Read journal files

    rect rgb(240, 240, 240)
        Note over CLI,Encoder: Export/JSON Rendering Flow
        CLI->>Encoder: NEW: Create Encoder with output options
        CLI->>FReader: Get entry
        FReader-->>CLI: Entry data
        CLI->>Encoder: NEW: AppendExport/AppendJSON/AppendJSONPretty
        Encoder->>DB: Read on-disk field order
        Encoder-->>CLI: Formatted output (systemd v262 compatible)
        alt Invalid field name (no '=')
            Encoder-->>CLI: Export: Stop rendering entry
        else Valid field name
            Encoder-->>CLI: JSON: Skip invalid field and continue
        end
    end

    rect rgb(240, 240, 240)
        Note over CLI,Boots: Boot Listing Flow (--list-boots)
        CLI->>DReader: Call ListBoots/list_boots
        DReader->>Boots: Query _BOOT_ID DATA arrays
        Boots->>DB: Read boot ID entry postings
        Boots-->>DReader: Boot ranges (first/last entries)
        alt Boots found
            DReader-->>CLI: All boots, oldest first
        else No _BOOT_ID DATA
            Boots->>DB: Scan entry headers
            Boots-->>DReader: Boot info from header boot IDs
            DReader-->>CLI: Boots listed (infallible for Rust DirectoryReader)
        end
        Note over Boots: NEW: No match/flush/position changes
    end

    Note over CLI,DReader: Public SDK helpers (both languages)<br/>DirectoryFiles, UniqueFiles, SipHash24,<br/>IsPrintableText, HeaderFlags

    Note over SDJ: NEW: SdJournal facade uses EntryEncoder for JSON<br/>with ShowAll (lossless values preserved)
Loading

Requires human review: Auto-approval blocked because this review re-detected 14 unresolved issues already reported by Cubic.

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

Comment thread go/journal/boots.go Resolved
Comment thread go/journal/entry_output_test.go Resolved
Comment thread rust/src/journal/src/boots.rs Resolved
Comment thread go/API.md Resolved
Comment thread rust/src/journal/src/export.rs Resolved
Comment thread .github/workflows/duplication.yml Resolved
Comment thread docs/Go-API.md Resolved
Lossless export now length-prefixes unsafe metadata and map-backed entries skip
fields with no values. Go ListBoots refreshes before reading postings so
long-lived readers see new entries. The duplication gate treats *_tests.rs as
test code; its workflow pins setup-node and drops persisted credentials.
Windows-only test failures are fixed, and docs state the fallback and JSON
parity accurately.
The follow loop repeated its emit and cursor-sync steps and exceeded the
complexity limit. The jscpd command is CI-provided and runs without a shell.

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