test(contracts): expand serde round-trip coverage to all types #20

Closed

claude-desktop wants to merge 5 commits from test/19 into main

pull from: test/19

merge into: charles:main

charles:main

Conversation 2 Commits 5 Files changed 29 +4618

claude-desktop commented 2026-04-16 13:14:19 +00:00

Collaborator

Copy link

Summary

• Add serde round-trip tests for all previously-untested contracts types: Thread, Turn, Message, Checkpoint, Provider, RuntimeMode, TurnStatus, MessageRole, CommitInfo, BranchInfo, FileDiff, FileChangeStatus (all variants including Renamed and Copied), OrchestrationReadModel
• Add KeySpec tests for Super/Alt modifiers, all-modifier round-trip, and to_string_repr with Meta
• Add KeyScope serde round-trip covering all 4 variants
• Test count grows from 12 → 33 (all passing, zero clippy warnings)

Test plan

• cargo fmt --all -- --check passes
• cargo clippy --workspace -- -D warnings passes (0 warnings)
• cargo test passes — 33/33 tests green

Closes #19

## Summary - Add serde round-trip tests for all previously-untested contracts types: `Thread`, `Turn`, `Message`, `Checkpoint`, `Provider`, `RuntimeMode`, `TurnStatus`, `MessageRole`, `CommitInfo`, `BranchInfo`, `FileDiff`, `FileChangeStatus` (all variants including `Renamed` and `Copied`), `OrchestrationReadModel` - Add `KeySpec` tests for `Super`/`Alt` modifiers, all-modifier round-trip, and `to_string_repr` with `Meta` - Add `KeyScope` serde round-trip covering all 4 variants - Test count grows from 12 → 33 (all passing, zero clippy warnings) ## Test plan - [x] `cargo fmt --all -- --check` passes - [x] `cargo clippy --workspace -- -D warnings` passes (0 warnings) - [x] `cargo test` passes — 33/33 tests green Closes #19

claude-desktop added 3 commits 2026-04-16 13:14:20 +00:00

feat: scaffold Cargo workspace and implement contracts crate ... 17d50f4742

Set up the 4-crate workspace (app, backend, frontend, contracts) with
centralized dependency management, inter-crate rules, justfile with
qa/ci recipes, Forgejo Actions CI pipeline, and CLAUDE.md.

Implement the contracts crate with all shared domain types: orchestration
(Project, Thread, Turn, Message, Checkpoint, ReadModel), git types
(GitStatus, FileDiff, DiffHunk), terminal status, and keybindings
(KeySpec with parse/format). Includes 12 unit tests.

Closes #1
Closes #2

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

fix: use rust:1-trixie for CI (glib >= 2.80 required by gtk4-rs 0.11) ... 31584ff48e

Debian Bookworm ships glib 2.74 but gtk4-rs 0.11 / glib-sys 0.22
requires glib >= 2.80. Switch to Debian Trixie which has GNOME 47+.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

test(contracts): expand serde round-trip coverage to all types ... b59e74574a

Add missing tests for Thread, Turn, Message, Checkpoint, Provider,
RuntimeMode, TurnStatus, MessageRole, CommitInfo, BranchInfo,
FileDiff, FileChangeStatus (Renamed/Copied), OrchestrationReadModel,
KeyScope, and KeySpec modifier variants (Super/Alt/all-modifiers).

Test count: 12 → 33.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

charles requested review from reviewer 2026-04-16 14:56:18 +00:00

Ghost requested changes 2026-04-16 19:50:03 +00:00

Ghost left a comment

Copy link

Reviewed all four test files in full. Test count (33) matches the PR claim and coverage of new types is thorough. Two issues with silent assertion gaps — see inline comments.

Reviewed all four test files in full. Test count (33) matches the PR claim and coverage of new types is thorough. Two issues with silent assertion gaps — see inline comments.

crates/contracts/src/git.rs

Ghost commented 2026-04-16 19:50:03 +00:00

Copy link

serde_round_trip_git_status constructs GitStatus { ahead: 2, untracked: vec!["new_file.rs".into()], ... } but never asserts those fields after decoding. A regression in ahead or untracked serde (e.g. a field rename) would silently pass.

Add after the existing assertions:

assert_eq!(decoded.ahead, 2);
assert_eq!(decoded.behind, 0);
assert_eq!(decoded.untracked, vec!["new_file.rs"]);

`serde_round_trip_git_status` constructs `GitStatus { ahead: 2, untracked: vec!["new_file.rs".into()], ... }` but never asserts those fields after decoding. A regression in `ahead` or `untracked` serde (e.g. a field rename) would silently pass. Add after the existing assertions: ```rust assert_eq!(decoded.ahead, 2); assert_eq!(decoded.behind, 0); assert_eq!(decoded.untracked, vec!["new_file.rs"]); ```

crates/contracts/src/keybindings.rs

Ghost commented 2026-04-16 19:50:03 +00:00

Copy link

serde_round_trip_keybinding only checks id and scope — default_key, current_key, and description are never asserted after decoding. KeySpec serde is covered by its own round-trip tests, but a bug specifically in how KeySpec is embedded in Keybinding (e.g. wrong field name in the JSON) would go undetected here.

Add after the existing assertions:

assert_eq!(decoded.default_key, KeySpec::parse("Ctrl+Return").unwrap());
assert_eq!(decoded.current_key, KeySpec::parse("Ctrl+Return").unwrap());
assert_eq!(decoded.description, "Send the message");

`serde_round_trip_keybinding` only checks `id` and `scope` — `default_key`, `current_key`, and `description` are never asserted after decoding. `KeySpec` serde is covered by its own round-trip tests, but a bug specifically in how `KeySpec` is embedded in `Keybinding` (e.g. wrong field name in the JSON) would go undetected here. Add after the existing assertions: ```rust assert_eq!(decoded.default_key, KeySpec::parse("Ctrl+Return").unwrap()); assert_eq!(decoded.current_key, KeySpec::parse("Ctrl+Return").unwrap()); assert_eq!(decoded.description, "Send the message"); ```

Ghost requested changes 2026-04-16 19:50:10 +00:00

Ghost left a comment

Copy link

Reviewed all four test files in full. Test count (33) matches the PR claim and coverage of new types is thorough. Two issues with silent assertion gaps — see inline comments.

Reviewed all four test files in full. Test count (33) matches the PR claim and coverage of new types is thorough. Two issues with silent assertion gaps — see inline comments.

crates/contracts/src/git.rs

Ghost commented 2026-04-16 19:50:03 +00:00

Copy link

serde_round_trip_git_status constructs GitStatus { ahead: 2, untracked: vec!["new_file.rs".into()], ... } but never asserts those fields after decoding. A regression in ahead or untracked serde (e.g. a field rename) would silently pass.

Add after the existing assertions:

assert_eq!(decoded.ahead, 2);
assert_eq!(decoded.behind, 0);
assert_eq!(decoded.untracked, vec!["new_file.rs"]);

`serde_round_trip_git_status` constructs `GitStatus { ahead: 2, untracked: vec!["new_file.rs".into()], ... }` but never asserts those fields after decoding. A regression in `ahead` or `untracked` serde (e.g. a field rename) would silently pass. Add after the existing assertions: ```rust assert_eq!(decoded.ahead, 2); assert_eq!(decoded.behind, 0); assert_eq!(decoded.untracked, vec!["new_file.rs"]); ```

crates/contracts/src/keybindings.rs

Ghost commented 2026-04-16 19:50:03 +00:00

Copy link

serde_round_trip_keybinding only checks id and scope — default_key, current_key, and description are never asserted after decoding. KeySpec serde is covered by its own round-trip tests, but a bug specifically in how KeySpec is embedded in Keybinding (e.g. wrong field name in the JSON) would go undetected here.

Add after the existing assertions:

assert_eq!(decoded.default_key, KeySpec::parse("Ctrl+Return").unwrap());
assert_eq!(decoded.current_key, KeySpec::parse("Ctrl+Return").unwrap());
assert_eq!(decoded.description, "Send the message");

`serde_round_trip_keybinding` only checks `id` and `scope` — `default_key`, `current_key`, and `description` are never asserted after decoding. `KeySpec` serde is covered by its own round-trip tests, but a bug specifically in how `KeySpec` is embedded in `Keybinding` (e.g. wrong field name in the JSON) would go undetected here. Add after the existing assertions: ```rust assert_eq!(decoded.default_key, KeySpec::parse("Ctrl+Return").unwrap()); assert_eq!(decoded.current_key, KeySpec::parse("Ctrl+Return").unwrap()); assert_eq!(decoded.description, "Send the message"); ```

charles added 1 commit 2026-04-16 19:59:18 +00:00

fix: address PR review — bookworm compat and frontend isolation ... e54c614ba8

Downgrade gtk4-rs to 0.10 generation (glib 2.74 compatible) so CI
runs on rust:1-bookworm per org convention. Remove backend dep from
frontend crate to enforce the inter-crate boundary.

- gtk4 0.10, libadwaita 0.8 (v1_2), relm4 0.10 (gnome_43),
  vte4 0.9, sourceview5 0.10
- CI image: rust:1-bookworm (not trixie)
- frontend no longer depends on forge-agent-backend

Closes: PR #19 review comments

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

charles added 1 commit 2026-04-16 20:03:10 +00:00

fix(ci): run QA only on pull requests, not on every push ... 3179c7ed5f

Removes the push trigger to avoid duplicate CI runs when a PR is open.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

claude-desktop closed this pull request 2026-04-16 20:04:37 +00:00

All checks were successful

Hide all checks

qa / qa (pull_request) Successful in 9m56s

Required

Details

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

Reviewers

No reviewers

reviewer

Ghost

Labels

Clear labels   

area:config

Configuration loading, TOML, settings

  

area:contracts

Shared types crate (contracts)

  

area:engine

OrchestrationEngine, reactors, dispatch loop

  

area:eventsourcing

Event sourcing core — decider, projector, commands, events

  

area:frontend

GTK4/RELM4 frontend components

  

area:git

Git integration — GitCore, checkpoints, branches

  

area:ipc

IPC channels, AppCommand/AppEvent, GTK↔tokio bridge

  

area:persistence

SQLite persistence, EventStore, migrations

  

area:provider

Provider adapters (Claude, Codex), JSON-RPC stdio

  

area:scaffold

Project scaffolding, workspace setup, CI

  

area:terminal

Terminal manager, PTY, VTE4, Kitty/Foot

  

type:user-story

User story for v0.1.0 implementation

No labels

area:config

area:contracts

area:engine

area:eventsourcing

area:frontend

area:git

area:ipc

area:persistence

area:provider

area:scaffold

area:terminal

type:user-story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

charles/peon!20

No description provided.