Skip to content

fix(cli): implement transparent serialization for JobId and fix receipt parsing#3066

Open
brbrainerd wants to merge 1 commit into
spacedriveapp:mainfrom
brbrainerd:fix/cli-job-serialization
Open

fix(cli): implement transparent serialization for JobId and fix receipt parsing#3066
brbrainerd wants to merge 1 commit into
spacedriveapp:mainfrom
brbrainerd:fix/cli-job-serialization

Conversation

@brbrainerd

@brbrainerd brbrainerd commented Apr 20, 2026

Copy link
Copy Markdown

This PR resolves CLI communication failures caused by improper JSON deserialization of Job identifiers.

Key changes:

  • Enables #[serde(transparent)] on JobId to ensure it serializes as a plain UUID string.
  • Updates index and file CLI domains to correctly parse JobReceipt objects returned by the daemon.

Task File: .tasks/core/CLI-001-fix-jobid-serialization-and-receipts.md

Closes CLI-001

@coderabbitai

coderabbitai Bot commented Apr 20, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: eb857875-9041-4db2-8104-c25d90b832bb

📥 Commits

Reviewing files that changed from the base of the PR and between ca16557 and 10e9edb.

📒 Files selected for processing (4)
  • .tasks/core/CLI-001-fix-jobid-serialization-and-receipts.md
  • apps/cli/src/domains/file/mod.rs
  • apps/cli/src/domains/index/mod.rs
  • core/src/infra/job/types.rs
✅ Files skipped from review due to trivial changes (1)
  • .tasks/core/CLI-001-fix-jobid-serialization-and-receipts.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • core/src/infra/job/types.rs
  • apps/cli/src/domains/index/mod.rs

Walkthrough

Adds #[serde(transparent)] to JobId so it serializes as a raw UUID; updates CLI index and file commands to accept and print JobReceipt (id + job_name) returned by the daemon; and adds a task doc describing the issue and acceptance criteria.

Changes

Cohort / File(s) Summary
Task Documentation
​.tasks/core/CLI-001-fix-jobid-serialization-and-receipts.md
Adds task record describing the JobId serde change and CLI receipt parsing/printing acceptance criteria.
Core Job Types
core/src/infra/job/types.rs
Added #[serde(transparent)] to pub struct JobId(pub Uuid); so JobId serializes/deserializes as a raw UUID string.
CLI File Domain
apps/cli/src/domains/file/mod.rs
Changed run_copy_with_confirmation return type to Result<JobReceipt>, updated imports to sd_core::infra::job::handle::JobReceipt, assigned execute_action! result to receipt, and print receipt.id and receipt.job_name.
CLI Index Domain
apps/cli/src/domains/index/mod.rs
Updated Index command handlers to expect JobReceipt from execute_action!, adjusted imports and print_output! callbacks to print r.id and r.job_name.

Sequence Diagram(s)

sequenceDiagram
    participant CLI
    participant Daemon
    participant Core

    CLI->>Daemon: submit job request (index / copy)
    Daemon->>Core: create job (generate JobId, set job_name)
    Core-->>Daemon: JobReceipt { id, job_name }
    Daemon-->>CLI: JSON-encoded JobReceipt
    CLI->>CLI: deserialize JobReceipt, print id and job_name
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A tiny tweak, transparent and bright,
JobIds now shine as UUIDs in flight,
Receipts arrive with name and id in tow,
The CLI prints both, and onward we go—
Hops of joy for tidy JSON flow! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description covers the key changes and includes a reference to the task file, but does not follow the required template which mandates a 'Closes #(issue)' section. Add a properly formatted issue reference matching the template: 'Closes #CLI-001' or update to use standard GitHub issue number format if CLI-001 is not a valid GitHub issue identifier.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main changes: enabling transparent serialization for JobId and fixing receipt parsing in the CLI.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

Comment @coderabbitai help to get the list of available commands and usage tips.

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

🧹 Nitpick comments (2)
apps/cli/src/domains/file/mod.rs (1)

3-14: Keep sd_core imports in the external-crate group.

Line 11 adds an external crate import after local crate imports. Move sd_core up with the other external crates. As per coding guidelines, **/*.rs: “Group imports with blank lines: standard library, external crates, then local modules”.

Proposed import grouping
 use anyhow::Result;
 use clap::Subcommand;
 use comfy_table::presets::UTF8_BORDERS_ONLY;
+use sd_core::infra::job::handle::JobReceipt;
+use sd_core::infra::query::LibraryQuery;
 
+use crate::context::Context;
 use crate::format_bytes;
 use crate::util::prelude::*;
-
-use crate::context::Context;
-use sd_core::infra::job::handle::JobReceipt;
-use sd_core::infra::query::LibraryQuery;
 
 use self::args::*;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/cli/src/domains/file/mod.rs` around lines 3 - 14, The sd_core imports
(sd_core::infra::job::handle::JobReceipt and
sd_core::infra::query::LibraryQuery) are placed after local crate imports; move
those two use lines into the external-crate group with the other externals
(e.g., after comfy_table::presets::UTF8_BORDERS_ONLY) and ensure a blank line
separates external crates from local crate imports (use crate::... and
self::args::* remain in the local group).
apps/cli/src/domains/index/mod.rs (1)

3-12: Move the sd_core import above local imports.

Line 10 adds an external crate import after local crate imports. Keep external crates together before local modules. As per coding guidelines, **/*.rs: “Group imports with blank lines: standard library, external crates, then local modules”.

Proposed import grouping
 use anyhow::Result;
 use clap::Subcommand;
 use comfy_table::{presets::UTF8_BORDERS_ONLY, Attribute, Cell, Table};
+use sd_core::{infra::job::handle::JobReceipt, ops::libraries::list::query::ListLibrariesQuery};
 
 use crate::util::prelude::*;
-
 use crate::{context::Context, util::error::CliError};
-use sd_core::{infra::job::handle::JobReceipt, ops::libraries::list::query::ListLibrariesQuery};
 
 use self::args::*;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/cli/src/domains/index/mod.rs` around lines 3 - 12, The sd_core external
crate import (sd_core::{infra::job::handle::JobReceipt,
ops::libraries::list::query::ListLibrariesQuery}) is placed after local crate
imports; move that line up into the external-crates block so external imports
appear together before local modules, and ensure a blank line separates external
crates from local imports (e.g., keep use clap::Subcommand and comfy_table...
and sd_core grouped together, then a blank line, then use crate::... and use
self::args::*).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@apps/cli/src/domains/file/mod.rs`:
- Around line 3-14: The sd_core imports (sd_core::infra::job::handle::JobReceipt
and sd_core::infra::query::LibraryQuery) are placed after local crate imports;
move those two use lines into the external-crate group with the other externals
(e.g., after comfy_table::presets::UTF8_BORDERS_ONLY) and ensure a blank line
separates external crates from local crate imports (use crate::... and
self::args::* remain in the local group).

In `@apps/cli/src/domains/index/mod.rs`:
- Around line 3-12: The sd_core external crate import
(sd_core::{infra::job::handle::JobReceipt,
ops::libraries::list::query::ListLibrariesQuery}) is placed after local crate
imports; move that line up into the external-crates block so external imports
appear together before local modules, and ensure a blank line separates external
crates from local imports (e.g., keep use clap::Subcommand and comfy_table...
and sd_core grouped together, then a blank line, then use crate::... and use
self::args::*).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 78193b71-6312-441b-858f-2b3337f01930

📥 Commits

Reviewing files that changed from the base of the PR and between 60369e9 and 6785fc9.

📒 Files selected for processing (4)
  • .tasks/core/CLI-001-fix-jobid-serialization-and-receipts.md
  • apps/cli/src/domains/file/mod.rs
  • apps/cli/src/domains/index/mod.rs
  • core/src/infra/job/types.rs

@brbrainerd brbrainerd force-pushed the fix/cli-job-serialization branch 2 times, most recently from ca16557 to 70ba74c Compare April 20, 2026 17:08
@brbrainerd brbrainerd force-pushed the fix/cli-job-serialization branch from 70ba74c to 10e9edb Compare April 20, 2026 17:10
@brbrainerd

Copy link
Copy Markdown
Author

Addressed automated review feedback: standardized import organization across the CLI file and index domains.

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