Repository navigation
Conversation
…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.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
There was a problem hiding this comment.
Pull Request Overview
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
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 278 |
| Duplication | -11 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
7 issues found and verified against the latest diff
Confidence score: 3/5
file_boot_rangesinboots.rscan return a posting beyond the reader’s visible tail. Bound the search by the last visible entry, as the Go implementation does.duplication.ymlcan 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.rsstill routes files with non-UTF-8 namespace parents toLOCAL_SYSTEM. Handle the parent path without assuming it is UTF-8.SOW-0176-20261108-single-ownership-followups.mdis 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
There was a problem hiding this comment.
1 issue found across 71 files
Confidence score: 3/5
boots.rscan list a boot that canonical_BOOT_IDmatching 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)
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
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.
Pending user review
Agent-chosen implementation choices (recorded in SOW-0097):
EntryEncoder(GoNewEntryEncoder+AppendExport/AppendJSON/AppendJSONPretty; RustEntryEncoder::new+append_*) and the public printable ruleIsPrintableText/is_printable_text. Follows systemd v262output_export/output_jsonexactly; JSON keys are sorted (systemd's order is hash order)._BOOT_IDDATA entry arrays (no match flush, no position change). Only canonical lowercase_BOOT_IDvalues name a boot, as the old CLIs effectively did. NewReader.ListBoots(Go) andFileReader::list_boots(Rust).Open decisions (tracked in SOW-0176; current behavior unchanged):
ExportEntry/export_entry_byteskeep the SOW-0043 lossless contract instead of becoming systemd-exact.DirectoryReader::list_boots/SdJournal::list_bootskeep the infallibleVecsignature (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:
--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
--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_IDDATA: Go 3.61 s -> 77 ms, Rust 1.50 s -> 46 ms.Validation
go test ./...andcargo test --workspacepass.--list-bootssweep 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(GoNewEntryEncoder, RustEntryEncoder::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_bootsreports every boot from_BOOT_IDDATA 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.ExportEntry/export_entry_byteskeep the lossless contract, now length-prefixing unsafe metadata and skipping fields with no values; open contract decisions are tracked in SOW-0176._BOOT_IDDATA 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.