fix(state): fix Errored(None) producing invalid TOML state file by jdx · Pull Request #231 · jdx/pitchfork

jdx · 2026-02-11T19:33:31Z

Summary

DaemonStatus::Errored(Option<i32>) with None serialized to an empty TOML table [status] because TOML has no null representation
This caused parse errors when reading the state file back, losing all daemon state and producing flaky test_stop_already_stopped failures
Changed Errored(Option<i32>) to Errored(i32) using -1 as sentinel for unknown exit codes
Added TOML roundtrip test to prevent regression

Test plan

New test_daemon_status_toml_roundtrip test passes for all variants
Existing test_daemon_status_json_roundtrip still passes
test_stop_already_stopped should no longer flake

🤖 Generated with Claude Code

Note

Low Risk
Small, localized state-model change plus formatting/test updates; primary risk is backward compatibility when deserializing previously persisted state files.

Overview
Fixes daemon state persistence by changing DaemonStatus::Errored from Option<i32> to an i32 exit code using -1 to represent an unknown code, avoiding invalid TOML output.

Updates lifecycle monitoring and TUI status/error rendering to treat -1 as “unknown”, and adds a TOML serialize/deserialize roundtrip test alongside the existing JSON roundtrip to prevent regression.

^{Written by Cursor Bugbot for commit d4acc2c. This will update automatically on new commits. Configure here.}

`DaemonStatus::Errored(Option<i32>)` with `None` serialized to an empty TOML table `[status]` because TOML has no null representation. This caused parse errors when reading the state file back, losing all daemon state and causing flaky test failures. Change `Errored(Option<i32>)` to `Errored(i32)` using -1 as sentinel for unknown exit codes. Add TOML roundtrip test to prevent regression. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

gemini-code-assist · 2026-02-11T19:33:50Z

Summary of Changes

Hello @jdx, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug where daemon state files, when containing an Errored(None) status, would produce invalid TOML due to TOML's lack of a null representation. This led to state file parse errors and intermittent test failures. The fix involves modifying the DaemonStatus::Errored enum variant to directly hold an i32 exit code, using -1 as a sentinel for unknown codes, thereby ensuring proper TOML serialization and deserialization.

Highlights

Enum Variant Refactoring: Refactored DaemonStatus::Errored to use i32 instead of Option<i32>, employing -1 as a sentinel for unknown exit codes.
TOML Serialization Fix: Resolved issues where Errored(None) serialized to an invalid TOML state, which previously caused parse errors and flaky test failures.
New Test Coverage: Introduced a new TOML roundtrip test for DaemonStatus to prevent future regressions in serialization and deserialization.

Changelog

src/daemon_status.rs
- Changed DaemonStatus::Errored from Option<i32> to i32 with a comment indicating -1 for unknown exit codes.
- Updated the error_message method to correctly handle the new Errored(i32) variant, distinguishing between known exit codes (>= 0) and unknown (-1).
- Refactored test data into a shared all_variants function to improve test maintainability.
- Removed a println! statement from test_daemon_status_json_roundtrip.
- Added test_daemon_status_toml_roundtrip to ensure correct TOML serialization and deserialization for all daemon status variants.
src/supervisor/lifecycle.rs
- Modified calls to DaemonStatus::Errored to pass i32 values, using -1 when the original exit code was None.
src/tui/ui.rs
- Adjusted the status_display function to correctly format the error message for DaemonStatus::Errored(i32), displaying the code if known (>= 0) or 'errored' if unknown (-1).

Activity

The new test_daemon_status_toml_roundtrip test passes for all DaemonStatus variants, confirming the fix.
The existing test_daemon_status_json_roundtrip continues to pass, ensuring no regressions in JSON serialization.
The test_stop_already_stopped test should no longer exhibit flakiness, indicating the underlying state corruption issue is resolved.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

Copilot

Pull request overview

Fixes invalid TOML state serialization for DaemonStatus::Errored when the exit code is unknown by replacing Option<i32> with a sentinel i32 value and adding a regression test for TOML round-tripping.

Changes:

Change DaemonStatus::Errored(Option<i32>) to DaemonStatus::Errored(i32) with -1 meaning “unknown exit code”.
Update UI + supervisor lifecycle code paths to use/display the new sentinel value.
Add TOML roundtrip test to prevent regressions in state file serialization.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
src/tui/ui.rs	Updates status display logic to hide sentinel/unknown exit codes.
src/supervisor/lifecycle.rs	Writes `Errored(-1)` when an exit code is unavailable.
src/daemon_status.rs	Updates enum + error message formatting and adds TOML roundtrip test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T19:34:29Z

src/daemon_status.rs

+    #[test]
+    fn test_daemon_status_toml_roundtrip() {
+        #[derive(Serialize, Deserialize, Debug)]
+        struct Wrapper {
+            status: DaemonStatus,
+        }


#[derive(Serialize, Deserialize)] requires Serialize/Deserialize to be in scope inside mod tests. Since use items from the parent module don’t automatically carry into child modules, this can fail to compile. Fix by importing serde::{Serialize, Deserialize} within mod tests, or by qualifying the derives as #[derive(serde::Serialize, serde::Deserialize, Debug)].

Copilot · 2026-02-11T19:34:30Z

src/supervisor/lifecycle.rs

+                        Some(code) => DaemonStatus::Errored(code),
+                        None => DaemonStatus::Errored(-1),


-1 is a magic number used as a sentinel for “unknown exit code” and is repeated across files (also in UI/tests). Introduce a single shared constant (e.g., DaemonStatus::UNKNOWN_EXIT_CODE or const UNKNOWN_EXIT_CODE: i32 = -1;) and use it everywhere to avoid accidental inconsistencies and make intent explicit.

Copilot · 2026-02-11T19:34:30Z

src/daemon_status.rs

-    #[test]
-    fn test_daemon_status_json_roundtrip() {
-        let variants = vec![
+    fn all_variants() -> Vec<(&'static str, DaemonStatus)> {


all_variants is a bit generic for a test helper; consider a more specific name like daemon_status_test_variants/status_variants_for_roundtrip to make it clearer that this is a test-only set used for serialization roundtrips.

gemini-code-assist

Code Review

This pull request effectively resolves the TOML serialization issue with DaemonStatus::Errored(None) by replacing Option<i32> with i32 and using -1 as a sentinel for an unknown exit code. The changes are consistently applied across the codebase, and the addition of a TOML roundtrip test is a great measure to prevent future regressions. The test refactoring to use a shared all_variants function is also a nice improvement for maintainability. I have one minor suggestion to improve code conciseness.

gemini-code-assist · 2026-02-11T19:34:41Z

src/supervisor/lifecycle.rs

                    let status = match status.code() {
-                        Some(code) => DaemonStatus::Errored(Some(code)),
-                        None => DaemonStatus::Errored(None),
+                        Some(code) => DaemonStatus::Errored(code),
+                        None => DaemonStatus::Errored(-1),
                    };


This match statement can be made more concise by using Option::unwrap_or.

let status = DaemonStatus::Errored(status.code().unwrap_or(-1));

When sysinfo reaps a process before tokio's child.wait(), the monitoring task gets an Err exit status. The error path was unconditionally setting Errored without checking if the daemon was being intentionally stopped, causing a race where it could overwrite the stop handler's Stopped status with Errored(-1). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is ON, but a Cloud Agent failed to start.}

src/daemon_status.rs

Negative exit codes are valid on Windows (e.g. STATUS_ACCESS_VIOLATION is 0xC0000005 = -1073741819 as i32). Only -1 is the sentinel for unknown, so check != -1 instead of >= 0. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T21:55:47Z

src/supervisor/lifecycle.rs

+            } else if is_stopping {
+                // Process was being intentionally stopped but child.wait() returned
+                // an error (e.g. due to sysinfo reaping the process first)
+                if let Err(e) = SUPERVISOR
+                    .upsert_daemon(UpsertDaemonOpts {
+                        id: id.clone(),
+                        pid: None,
+                        status: DaemonStatus::Stopped,
+                        last_exit_success: Some(true),
+                        ..Default::default()
+                    })
+                    .await
+                {
+                    error!("Failed to update daemon state for {id}: {e}");
+                }


The new code path for handling intentional stops when child.wait() returns an error lacks test coverage. Consider adding a test case that simulates this scenario to ensure the daemon status is correctly set to Stopped with last_exit_success: true.

## 🤖 New release * `pitchfork-cli`: 1.4.1 -> 1.4.2 <details><summary>Changelog</summary> <blockquote> ## [1.4.2](v1.4.1...v1.4.2) - 2026-02-11 ### Fixed - *(state)* fix Errored(None) producing invalid TOML state file ([#231](#231)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/).  --- > [!NOTE] > **Low Risk** > Release metadata-only changes (version/changelog/lockfile) with no functional code modifications in this PR. > > **Overview** > Prepares the `v1.4.2` release by bumping `pitchfork-cli` version from `1.4.1` to `1.4.2` in `Cargo.toml`/`Cargo.lock`. > > Updates `CHANGELOG.md` with the `1.4.2` entry noting a state serialization fix for `Errored(None)` producing invalid TOML. > > Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 04df47a. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).

Copilot AI review requested due to automatic review settings February 11, 2026 19:33

Copilot AI reviewed Feb 11, 2026

View reviewed changes

gemini-code-assist bot reviewed Feb 11, 2026

View reviewed changes

cursor bot reviewed Feb 11, 2026

View reviewed changes

src/daemon_status.rs Show resolved Hide resolved

Copilot AI review requested due to automatic review settings February 11, 2026 21:55

Copilot AI reviewed Feb 11, 2026

View reviewed changes

jdx merged commit b104a60 into main Feb 11, 2026
5 checks passed

jdx deleted the fix/errored-none-toml-serialization branch February 11, 2026 22:00

jdx mentioned this pull request Feb 11, 2026

chore: release v1.4.2 #230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(state): fix Errored(None) producing invalid TOML state file#231

fix(state): fix Errored(None) producing invalid TOML state file#231
jdx merged 3 commits intomainfrom
fix/errored-none-toml-serialization

jdx commented Feb 11, 2026 •

edited by cursor bot

Loading

Uh oh!

gemini-code-assist bot commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Feb 11, 2026

Uh oh!

cursor bot left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		Some(code) => DaemonStatus::Errored(code),
		None => DaemonStatus::Errored(-1),

Conversation

jdx commented Feb 11, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

gemini-code-assist bot commented Feb 11, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jdx commented Feb 11, 2026 •

edited by cursor bot

Loading