feat!: emit MaybeUndefined<T> for nullable input fields by flipbit03 · Pull Request #141 · flipbit03/lineark

flipbit03 · 2026-04-16T12:19:50Z

Closes #139.
Closes #142.
Closes #143.

This PR started as the MaybeUndefined<T> work for #139 and grew into three related codegen improvements as the conversation surfaced adjacent gaps. Each is its own commit so reviewers can read the progression.

Commit-by-commit

feat!: emit MaybeUndefined<T> for nullable input fields (codegen: emit MaybeUndefined<T> for nullable input fields to support explicit null #139)
GraphQL distinguishes "omit this field" from "send explicit null". Generated *Input structs flattened both into Option<T> + skip_serializing_if, forcing CLI handlers to bypass the typed mutation, hand-roll the GraphQL string, and inject serde_json::Value::Null to clear a field. Three sites had this workaround already (issues update --clear-parent, projects update --clear-{lead,start-date,target-date}).

Codegen now emits nullable input fields as MaybeUndefined<T> (Undefined / Null / Value) and required fields as plain T. The --clear-* paths collapse to MaybeUndefined::Null, and every hand-rolled mutation string and JSON-null injection in crates/lineark/src/commands/ is gone. The CLI is now fully reliant on the codegen'd SDK — no escape hatches.
codegen(inputs): recurse on nullability instead of panicking (codegen: handle nullable-inner list types ([T], [T]!) without panicking #142, input side)
First iteration panicked on [T] / [T]! shapes because Linear's schema has none today. Replaced with uniform recursion: outermost nullable wraps in MaybeUndefined<T>, deeper nullable wraps in Option<T>, lists wrap their element. Handles every list shape the GraphQL spec allows. Generated inputs.rs byte-identical for Linear's current schema.
codegen(types): honor list element nullability for outputs (codegen: handle nullable-inner list types ([T], [T]!) without panicking #142, output side)
Output codegen was silently flattening [T] into Vec<T> (a runtime deserialize-fail waiting to happen). Same uniform-recursion rule applied to outputs. Generated types.rs byte-identical for Linear's current schema.
test(codegen): cover every list shape with synthetic schemas (test coverage for codegen: handle nullable-inner list types ([T], [T]!) without panicking #142)
Linear has zero [T] / [T]! shapes, so regeneration alone doesn't exercise the new code paths. Added unit tests in both emitters that build synthetic GqlType trees for every shape (T!, T, [T!]!, [T!], [T]!, [T], plus nesting like [[T!]!]!) and assert the rendered Rust types directly.
codegen: only Box fields that actually create size cycles (codegen: only Box fields that actually create size cycles #143)
Replaced unconditional Box<T> on Object/InputObject fields with a per-field cycle analysis: build a reference graph excluding list edges, compute reachability, Box only when target can reach container. Rust's sizedness checker is the ground truth — if the analysis misses a cycle, cargo build fails.
- Box<> in inputs.rs: 1094 → 210 (-81%)
- Box<> in types.rs: 1070 → 182 (-83%)
address review feedback: docs, offline contract tests, dedup, 3-node cycle
Follow-ups from an independent review: doc the struct-context contract explicitly, doc the intentional asymmetry between From<Option<T>> (None → Undefined) and Deserialize (JSON null → Null), doc the conditional Eq + Hash bounds. Three new offline tests pin the Null → JSON null contract end-to-end through a real generated ProjectUpdateInput. Dedupe edges in compute_*_reachability via HashSet. Add a 3-node cycle test (A→B→C→A) plus an Outsider upstream of the cycle to verify non-cyclic nodes don't get spuriously Boxed.
lint: collapse nested guards in emit_mutations match arms
Rust 1.95's clippy added collapsible_match as an error under -D warnings. Flattened two pre-existing match X => { if <guard> { ... } } patterns in emit_mutations.rs. Semantically identical, regenerated SDK byte-identical.

Breaking change (bump to v3.0.0)

Latest release is v2.1.2. This PR is a breaking change for downstream SDK consumers — *Input field types shifted from Option<T> to MaybeUndefined<T> (nullable) or T (required) — so the next release should be v3.0.0 per semver.

Migration is mechanical:

Some(x) on a nullable field → x.into()
Some(x) on a required field → drop the Some
None fields → omit entirely (the Default fills in MaybeUndefined::Undefined)

The same mechanical rules were applied to the test files in this PR, so every call-site already demonstrates the migration pattern.

Test plan

make check — fmt, clippy (Rust 1.95), doc, build all clean
make test — 235 offline tests pass (47 SDK + 65 lint + 123 CLI)
make test-online — 107 online tests pass against live Linear API (40 SDK + 67 CLI), including projects_update_multiple_fields_and_clear_lead and issues_create_with_parent_and_clear_parent unmodified
49 codegen unit tests pass, including synthetic shape/cycle coverage
No serde_json::Value::Null remains under crates/lineark/src/commands/
No hand-written mutation(...) strings remain under crates/lineark/src/commands/
Box<> reduction in generated SDK: inputs.rs 1094 → 210 (-81%), types.rs 1070 → 182 (-83%)

GraphQL distinguishes "omitted" from "explicit null" on input fields, but generated `*Input` structs flattened both into `Option<T>` + `skip_serializing_if`. CLI handlers had to bypass the typed mutation, hand-roll the GraphQL string, and inject `serde_json::Value::Null` to clear a field. Three sites had this workaround already (`issues update --clear-parent`, `projects update --clear-{lead,start-date,target-date}`), and every new clear flag would add another. Codegen now emits nullable fields as `MaybeUndefined<T>` (Undefined / Null / Value) and required fields as plain `T`. The `--clear-*` paths collapse to `MaybeUndefined::Null`, and all hand-rolled mutation strings are gone. Breaking change for SDK consumers — pre-1.0, `From<T>` and `From<Option<T>>` make migration mechanical (`Some(x)` → `x.into()`, required fields drop the `Some`). Closes #139

Replace the `[T]` / `[T]!` panic with uniform recursion: at the outermost position nullable wraps in `MaybeUndefined<T>`; inside any list, nullable wraps in `Option<T>`; required positions stay plain at any depth. GraphQL list slots are always materialized on the wire, so the three-state Undefined/Null/Value distinction is meaningless one level in — `Option` is the right shape there. This handles every list shape the spec allows, including nested ones like `[[T!]!]!`, without any schema-specific guard. Generated `inputs.rs` is byte-identical for Linear's current schema (zero nullable-inner lists). The change is purely defensive: future schema additions are handled instead of crashing the build.

Apply the same uniform-recursion rule to output types: nullable elements inside a list become `Option<T>` instead of being silently flattened to `T`. The outermost field is still always wrapped in `Option<T>` (the lean-struct selection convention — consumers can omit fields they don't select), and Object types still get `Box` for mutual-recursion sizing. Closes the input/output asymmetry: both code paths now handle every list shape the GraphQL spec allows (`[T!]!`, `[T!]`, `[T]!`, `[T]`, and any nesting like `[[T!]!]!`) without special-casing the schema. Generated `types.rs` is byte-identical for Linear's current schema (zero nullable-inner lists across the entire SDL).

Linear's schema has zero nullable-inner lists, so the new recursive nullability rules in emit_inputs.rs and emit_types.rs are correct but unexercised by the regenerated output. Add unit tests that build synthetic GqlType trees for every shape the GraphQL spec allows (T, T!, [T!]!, [T!], [T]!, [T], plus nested forms like [[T!]!]! and [[T]!]) and assert the resolved Rust types directly. Also locks in the existing convention that the outer wrap of an Object field always uses Box — even for list-of-Object fields where the Vec already lives on the heap. Captured so any future change to that rule is intentional.

Previously every Object/InputObject field was wrapped in `Box<T>` unconditionally — a safe over-approximation that prevented infinite-size Rust structs but added a heap allocation per field per instance, even for the vast majority of fields where no cycle exists. Replace with a precise per-field analysis: 1. Build a directed reference graph excluding edges through lists, since `Vec<T>` is already heap-allocated and breaks size cycles regardless of `T`. 2. Compute transitive reachability once per emit pass. 3. For each field `Container.f: Target` that's a direct (non-list) reference, emit `Box<Target>` only when `Target` can transitively reach `Container` — exactly the shape that would otherwise produce an infinite-size struct. The rule applies symmetrically to inputs and outputs via a shared `dep_graph` module. For inputs, `Box` is also gated on `depth == 0`: inside a list the wrapper is `Vec<T>`, which is sized and never needs `Box` on its element. Effect on Linear's regenerated SDK: - `Box<>` in inputs.rs: 1094 → 210 (-81%) - `Box<>` in types.rs: 1070 → 182 (-83%) Verified by: - Workspace builds cleanly (Rust's sizedness checker confirms the remaining boxes are sufficient — if the analysis missed a cycle, rustc would reject the generated code). - All 232 offline + 107 online tests pass unchanged. - New synthetic unit tests (`box_only_when_field_creates_size_cycle` on both emitters) exercise cycle / non-cycle / list / self-loop cases independent of Linear's schema.

…cycle Follow-ups from an independent review of #141: - field_update.rs: document the struct-context contract explicitly (`MaybeUndefined::Undefined` only disappears from the wire when paired with the skip_serializing_if predicate on a struct field), the intentional asymmetry between `From<Option<T>>` (None → Undefined) and Deserialize (JSON null → Null), and the conditional Eq/Hash bounds. - tests/offline.rs: three new tests pin the central wire-format contract end-to-end through a real generated `ProjectUpdateInput`: Null → JSON null, Value → wrapped value, and a round-trip that verifies both Null and Undefined survive serialize+deserialize. Previously the contract was only exercised online via the --clear-* flag tests; now offline CI catches any codegen regression directly. - codegen: dedup edges in compute_{input,object}_reachability via HashSet to clarify intent (correctness was already guaranteed by the DFS's visited set, but the old Vec allowed duplicates). - codegen: add a three-node cycle test (A→B→C→A) plus an Outsider that points INTO the cycle. Catches bugs specific to longer cycles that the existing 2-node mutual-recursion test wouldn't, and verifies that upstream nodes don't get spuriously Boxed just because they can reach a cyclic node.

Rust 1.95's clippy added `collapsible_match` as an error under `-D warnings`. Two pre-existing match arms in emit_mutations.rs had a `match X => { if <guard> { ... } }` pattern that the lint wants flattened into `match X if <guard> => { ... }`. Behavior is identical: if the guard fails, the arm is skipped and the trailing `_ => {}` catches the fallthrough — same as the original inner `if`. Regenerated SDK is byte-identical.

Cargo.toml carries a `0.0.0` dev sentinel, so nothing in the tree reveals the real release version. Without checking, it's easy to reason as if the project is pre-1.0 when it's actually past v2.1.2, and size a breaking change accordingly in a PR description. Add a rule to always run `gh release list --limit 5` before reasoning about version bumps and to call out which major version the next release should be.

Ran `/update-linear-schema` on top of the cycle-aware codegen to stress-test the new analysis against a larger, drifted schema. Schema growth since the last sync: - enums: 71 → 88 (+17) - objects: 425 → 499 (+74) - inputs: 341 → 358 (+17) - query fields: 145 → 149 (+4) - mutation fields: 346 → 348 (+2) Zero hand-editing required: - The workspace compiles cleanly (Rust's sizedness checker would've rejected the regenerated types if the cycle-aware Box analysis missed any cycle through the +74 objects or +17 inputs — it didn't). - Zero `[T]` / `[T]!` shapes in the new schema, so the uniform-recursion rule kept producing correct Rust for every list shape without ever reaching the fallback path. - `make check && make test` clean (235 offline tests pass). No CLI commands referenced removed or renamed fields. No `EXCLUDED_FIELDS` adjustments needed.

…eate Linear's API has a transient failure mode where `*Create` mutations commit server-side but return "conflict on insert" to the client. The existing `run_lineark_with_retry` helper retries the same request body, which is counterproductive — Linear's content-hash idempotency keeps returning the same conflict, so every retry fails the same way. CI's Tests (Online) job has been flaking on this for at least the last several PRs (#140 hit it before the work in #141 even started). When we see "conflict on insert", read the entity back by name instead of retrying. The entity is already in Linear's database; we just need its post-create state. Both `*Create` and `read` responses share the shape that downstream test assertions care about (`id`, `name`, full details), so the synthesized response is interchangeable. Limited to `projects` for now since `projects read` accepts both UUIDs and names. `issues read` only accepts identifier/UUID; the one issues caller of this helper would need a search-and-filter recovery path that's bigger than this fix.

CI's Tests (Online) job has been flaking on `projects create` for several PRs. Linear's API has a transient failure mode where `*Create` mutations return "Entity X with id <UUID> already exists" for a UUID that doesn't actually exist server-side (verified by `read` immediately returning "Entity not found"). The same pattern hit PR #140's post-merge CI before the work in #141 even started. Local probing shows a clear cold-start signature: from a fresh process, the first ~3 attempts at `projects create` fail back-to-back, then it abruptly starts working (next 7 succeed in a row). The previous helper's 3-retry, 14s-backoff budget routinely runs out before the cold window closes. Layered fix: 1. **`run_lineark_with_retry`** (in tests/online.rs): - Up to 8 attempts with `[0, 2, 5, 10, 20, 30, 45, 60]`s backoff (~172s worst case) so we ride out the cold window. - Mutate the request body each retry by appending a fresh suffix to the `<name>` positional. Linear's stuck UUIDs are keyed on body content, so a different body avoids the cached state. - Returns `(Output, String)` where the second element is the name actually used. Callers shadow their `unique_name` with this so downstream `read by name` lookups match server state. 2. **CI workflow + Makefile** (`ci.yml`, `ci-online-fork.yml`, `Makefile`): wrap the whole online suite in a 3x retry with a workspace clean between attempts. Belt-and-suspenders for the occasional case where even the per-call helper's budget isn't enough.

…144) * fix(test): defeat Linear API's transient "conflict on insert" flake CI's Tests (Online) job has been flaking on `projects create` for several PRs. Linear's API has a transient failure mode where `*Create` mutations return "Entity X with id <UUID> already exists" for a UUID that doesn't actually exist server-side (verified by `read` immediately returning "Entity not found"). The same pattern hit PR #140's post-merge CI before the work in #141 even started. Local probing shows a clear cold-start signature: from a fresh process, the first ~3 attempts at `projects create` fail back-to-back, then it abruptly starts working (next 7 succeed in a row). The previous helper's 3-retry, 14s-backoff budget routinely runs out before the cold window closes. Layered fix: 1. **`run_lineark_with_retry`** (in tests/online.rs): - Up to 8 attempts with `[0, 2, 5, 10, 20, 30, 45, 60]`s backoff (~172s worst case) so we ride out the cold window. - Mutate the request body each retry by appending a fresh suffix to the `<name>` positional. Linear's stuck UUIDs are keyed on body content, so a different body avoids the cached state. - Returns `(Output, String)` where the second element is the name actually used. Callers shadow their `unique_name` with this so downstream `read by name` lookups match server state. 2. **CI workflow + Makefile** (`ci.yml`, `ci-online-fork.yml`, `Makefile`): wrap the whole online suite in a 3x retry with a workspace clean between attempts. Belt-and-suspenders for the occasional case where even the per-call helper's budget isn't enough. * ci: mark Tests (Online) as continue-on-error while Linear API flakes Linear's GraphQL API is returning spurious "conflict on insert" errors and occasional HTTP 502s on `projects create` today — reproducible with a single ad-hoc probe, not just under CI load. No amount of client-side retry can beat a degraded upstream, and the suite-level 3x retry in the previous commit still wasn't enough. Flip the step to `continue-on-error: true` so the online suite still runs (failures stay visible in the job log for triage) but doesn't gate the PR check. Revisit once Linear's API stabilizes, or move online tests to a separate non-required workflow. * docs(claude): structural guidelines for online tests against Linear's API Linear's GraphQL API has permanent consistency issues — phantom "conflict on insert" errors on creates, eventually-consistent search indexes, occasional 502s. These aren't transient; they're part of the environment. Online tests must be written defensively around them or they'll flake in CI regardless of our code. Replace the wall-of-text paragraph with a structured "three pillars" section that names the mandatory patterns and points at the helpers that implement each one: 1. Proactive cleanup — cleanup_zombies + RAII guards + create_test_team. 2. Unique names per attempt — UUID-suffixed names; retry helpers MUST mutate the body between attempts (run_lineark_with_retry appends a fresh suffix and returns the actually-used name). 3. Retries on every transient-capable call — retry_create for mutations, retry_search / retry_with_backoff for reads-after-create, settle() for propagation waits. Note the CI-level 3x suite retry as belt-and-suspenders, not a substitute. Also pins down the "check exit before parsing output" and "no-production-tokens" rules that were implicit. * test(online): align every test with the three pillars Audit-driven sweep of every online test against the cleanup / unique-name / retry pillars now codified in CLAUDE.md. Result: 100% pass on a fresh workspace (67 CLI + 40 SDK = 107 tests, ~5 min runtime). Pillar 1 (proactive cleanup): - `cleanup_workspace` was silently missing archived/trashed resources for issues, documents, projects, labels, and teams — the list calls didn't pass `include_archived(true)`. That left zombies accumulating across runs (we found 10 stuck-in-trash projects + 24 zombie teams in a single workspace). Fix: pass `include_archived(true)` on every list, and document why in the helper. Pillar 2 (unique names per attempt): - 6 SDK tests had hardcoded titles like `"[test] SDK issue_create_and_delete"` with no UUID suffix. Two test runs would collide. Added `Uuid::new_v4()[..8]` suffixes to each. Pillar 3 (retries on every transient-capable call): - 32 CLI tests + the `create_two_issues` helper called `lineark create` directly via `lineark().args(...).output()`, bypassing `run_lineark_with_retry`. Converted all 30+ sites; preserved `unique_name` shadowing where downstream lookups need the actually-used name. - `run_lineark_with_retry` bumped from 8 → 15 attempts with backoffs `[0,2,5,10,20,30,60×9]` (~9 min worst case). Body mutation gated on the `[test]` prefix so `comments create <uuid>` and `relations create <uuid>` positionals aren't corrupted. - `retry_create` in `lineark-test-utils` bumped from 3 → 15 attempts to match the CLI helper. Per-test persistence is the right pattern; re-running the whole suite for one bad test wastes cycles. Suite-level retry removed: - The CI workflows + Makefile previously wrapped the suite in a 3x retry + `continue-on-error: true`. Both gone. Per-call retries are persistent enough that any test failure is a real regression worth blocking on. * fix(fmt): apply rustfmt to retry.rs eprintln * test(infra): rotate test workspaces from a token pool `~/.linear_api_token_test` (and the `LINEAR_TEST_TOKEN` GitHub secret) now accepts a `;`-separated pool of API tokens — one per workspace. `test_token()` picks one at random per test process and pins it for the process lifetime via `OnceLock`, so a single test run stays consistent while consecutive runs spread across the pool. Why per-process and not per-test: a single test session creates teams + projects + issues that reference each other; mid-run token swap would orphan resources. The load-spreading benefit comes from the *number of runs over time*, not granularity within one run. Format details: - `;` is the primary separator. Newlines also separate (so single-line and one-per-line files both work). - Trailing `;` is harmless (`tok;` parses as one token). - Lines whose first non-whitespace is `#` are comments. Important: comment filtering happens *before* `;`-splitting, so a `;` inside a comment doesn't accidentally produce fake "tokens" — caught with a unit test. - Backwards compatible: a single-line single-token file still works. Logs the chosen index/N and the last 6 chars of the token at process start, so a CI-failure triage can identify which workspace was hit without dumping full secrets. 7 unit tests cover the parser. Verified locally: 100% green online suite (67 CLI + 40 SDK) with 3-workspace pool active. * test(infra): cleanup-test-workspace iterates the entire token pool Previously the cleanup binary called `test_token()` and cleaned only the one randomly-chosen workspace. With token rotation in place, that meant the explicit CI cleanup step ran on a *different* workspace than the test process drew — useless ceremony. The other workspaces silently accumulated zombies until they happened to lose three random draws in a row and the in-process `cleanup_zombies()` finally caught up. New behaviour: the binary loads every token from the pool via `all_test_tokens()` (newly exported from `lineark-test-utils`) and calls `cleanup_workspace` against each, with per-workspace failures isolated so one bad workspace doesn't block the rest. Logs a `cleanup: workspace N/total (token …<last6>)` line per pass — same triage tag the `test_token` log uses, so a failure lines up visually with which workspace the actual tests then picked. Verified locally: 100% green online suite (67 CLI + 40 SDK), three cleanup messages in the log show all pooled workspaces visited before the test phase began. * docs(claude): sync test-pillar guide with the final retry/CI shape Earlier in this PR I added the three-pillars CLAUDE.md section pointing at the helpers as they existed at that commit. Subsequent commits in the same PR bumped retry counts (3→15 for retry_create, 8→15 for the CLI helper), removed the suite-level 3x retry, and removed `continue-on-error: true` — but CLAUDE.md was never refreshed and ended up advertising state that no longer exists. Caught by an independent review. Updates: - Pillar 3: corrected attempt counts and backoff schedule for both helpers; called out that body mutation is gated on the `[test]` prefix (so UUID-positional commands like `comments create <uuid>` / `relations create <uuid>` aren't corrupted on retry). - "CI-level belt-and-suspenders" bullet replaced with "No suite-level retry" — explains why and notes the historical context. - New bullet under "Other online-test rules" documenting the `;`-separated multi-workspace token pool, `OnceLock` per-process pin, and the `cleanup-test-workspace` binary's whole-pool sweep.

flipbit03 added 9 commits April 16, 2026 09:19

flipbit03 merged commit 0ee74b2 into main Apr 17, 2026
15 of 16 checks passed

flipbit03 deleted the feat/139-codegen-emit-maybeundefined-t-for-nullab branch April 17, 2026 01:25

flipbit03 mentioned this pull request Apr 17, 2026

fix(test): defeat Linear API's transient "conflict on insert" flake #144

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat!: emit MaybeUndefined<T> for nullable input fields#141

feat!: emit MaybeUndefined<T> for nullable input fields#141
flipbit03 merged 9 commits into
mainfrom
feat/139-codegen-emit-maybeundefined-t-for-nullab

flipbit03 commented Apr 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

flipbit03 commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Commit-by-commit

Breaking change (bump to v3.0.0)

Test plan

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

flipbit03 commented Apr 16, 2026 •

edited

Loading