Skip to content

fix: setup team persistence, interactive pager, blocked-by relations#37

Merged
Finesssee merged 2 commits into
masterfrom
fix/setup-config-relations-34-35-36
Jul 18, 2026
Merged

fix: setup team persistence, interactive pager, blocked-by relations#37
Finesssee merged 2 commits into
masterfrom
fix/setup-config-relations-34-35-36

Conversation

@Finesssee

@Finesssee Finesssee commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test plan

  • Unit tests for relation endpoint swap and blocked-by API mapping
  • Unit tests for config keys / default_team TOML roundtrip
  • Unit tests for pager disabled on interactive commands
  • CLI help tests for config default-team and relations blocked-by
  • Manual: linear setup on a large workspace — full team list, team saved, terminal still echoes after exit
  • Manual: linear config set default-team ENG then i create without -t
  • Manual: relations add A B -r blocked-by creates B blocks A (same as blocks with swapped args)

Closes #34
Closes #35
Closes #36


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

…lations

Persist default team from setup/config (#35), disable pager for interactive
commands and paginate all teams in setup (#34), and map blocked-by to inverted
blocks so Linear accepts the relation (#36).

Closes #34
Closes #35
Closes #36
@Finesssee

Copy link
Copy Markdown
Collaborator Author

Thermo-nuclear review: PR #37

PR: #37
Scope: setup team persistence (#35), interactive pager + team pagination (#34), blocked-by relations (#36)
Verdict: FOLLOW_UP_REQUIRED — do not merge as-is


What's solid


Blockers

1. src/config.rs crosses 1000 lines (724 → 1001)

This is exactly the skill's hard smell: a PR that pushes a file past 1k.

The new surface (get_default_team / set_default_team / key listing / tests) belongs in config, but the file was already a mixed bag of IO, OAuth, keyring, workspace CRUD, and CLI key dispatch. This PR made that worse by ~280 lines of behavior + tests.

Code judo: extract before merge, not after:

  • src/config/mod.rs — load/save, Config / Workspace types
  • src/config/team.rs (or defaults.rs) — default-team get/set + key table
  • src/config/oauth.rs — already-adjacent OAuth helpers
  • keep tests next to each module

Or at minimum: pull default-team + known-keys into a focused submodule so config.rs stays under 1k.

2. src/main.rs grew further (2314 → 2651) and still owns setup

handle_setup is a full product command living in the binary root, and this PR made it longer (pagination, selection-by-key, persistence messaging, JSON shape). main.rs was already a god file; adding more wizard logic is structural debt, not a fix.

Code judo: move setup into src/commands/setup.rs (same pattern as doctor, auth, interactive). Pager policy can stay as a thin fn command_disables_pager in main, or better: a method on Commands / trait so main doesn't grow a parallel command taxonomy.

ust // preferred shape impl Commands { fn disables_pager(&self) -> bool { matches!(self, Self::Setup | ...) } }

3. Workspace { ..., default_team: None } splat is spaghetti growth

Every construction site now hand-writes default_team: None. That is the textbook "special case bolted into every call site" pattern. Miss one later and you get a compile error today — good — but the design is still noisy and will keep growing as more workspace fields appear.

Code judo: give Workspace a constructor / Default-friendly shape:

ust impl Workspace { fn with_api_key(api_key: impl Into<String>) -> Self { Self { api_key: api_key.into(), oauth: None, default_team: None } } }

Then set_api_key / workspace_add / OAuth insert sites stop re-listing every field. Preserve-on-update should load the existing workspace and mutate fields, not rebuild with partial clones of oauth/team.

4. Default-team resolution is only half-wired (product/architecture leak)

get_default_team is used in:

  • issues create
  • interactive

…and nowhere else that already requires --team (sprint, statuses, cycles, metrics, import, templates create, triage, etc.). The user-facing promise of "I set a default team in setup" is false for most team-scoped commands.

That is not a nit — it is incomplete design. Either:

  • A. Make default team a global fallback used by every optional/required team flag (canonical helper: resolve team = CLI arg → env → config), or
  • B. Document that default team only applies to create/interactive and change the setup copy accordingly.

Right now we have A advertised, B implemented. That is a boundary leak between config and command layer.


High-conviction structural issues (not cosmetic)

5. Relation success printing re-implements invert logic

resolve_relation_endpoints already encodes the swap. The human message then re-branches on is_inverted() to pick response fields. That is duplicated model knowledge.

Simpler (behavior-preserving for the common case):

ust println!( "Created relation: {} {} {}", from, relation.display_name(), to );

If you want API-canonical identifiers (LIN-1 not raw UUID args), un-invert with the same helper instead of a second if-ladder:

ust let (issue_id, related_id, _) = resolve_relation_endpoints(from, relation, to); // then map response issue/relatedIssue back using that knowledge once

Prefer deleting the second invert branch entirely.

6. Teams-fetch pagination is copy-pasted twice

Setup and interactive both embed nearly identical GraphQL + paginate_nodes boilerplate. Canonical home is api (or a small teams helper used by both):

ust pub async fn fetch_all_teams(client: &LinearClient) -> Result<Vec<Value>>

Setup should call that; interactive should call that. One query string, one page-size policy.

7. Profile resolution is reimplemented again

get_default_team / set_default_team hand-roll:

ext LINEAR_CLI_PROFILE → config.current → "default"

…which already exists as current_profile() (Result) and is repeated across api-key paths. set_default_team can also create an empty workspace (api_key: "") if nothing is configured — half-applied state when the user hasn't authenticated yet.

Prefer: require an existing profile/workspace (error if none), reuse one profile resolver, mutate in place.

8. config get default-team vs config show disagree on semantics

  • geteffective value (env wins via get_default_team)
  • showstored workspace value + separate env line

That dual model will confuse scripts and humans. Pick one:

  • get = stored, document env as override only, or
  • show/get both print effective with source annotation

Smaller maintainability notes (only if blockers fixed)

  • command_disables_pager treating Doctor as interactive is debatable — doctor is mostly output, not password/TUI. Prefer pager-off only for true terminal owners (setup/auth/interactive/config set-key). Mild.
  • Setup still prints output-format tips without persisting format — fine for Configuring does not persist team settings #35 scope, but the wizard still over-promises "Step 3" as configuration. Either persist or drop the step.
  • to_api_string(BlockedBy) == "blocks" without swap is footgun if reused outside resolve_relation_endpoints. Consider making API type + endpoints one function only, and not exposing a misleading to_api_string for inverted variants.

What I would merge toward (minimal restructuring path)

  1. Extract handle_setupcommands/setup.rs (main.rs stops growing).
  2. Extract default-team + known keys so config.rs is back under 1k (module split).
  3. Add Workspace::with_api_key / mutate-in-place updates; delete field-splat sites.
  4. Add api::fetch_all_teams; delete duplicate queries.
  5. Wire default team through a single resolve_team_arg(cli: Option<String>) -> Option<String> used by all team-flagged commands, or narrow the advertised contract.
  6. Collapse relation display invert branch.
  7. Keep the behavioral fixes for Terminal output is disabled from some commands, and not all teams are returned #34/Configuring does not persist team settings #35/relations add --relation blocked-by returns HTTP 400 (advertised but unsupported; should map to inverse 'blocks') #36 — they are right; the structure around them is not yet merge-grade.

Approval bar check

Criterion Status
No clear structural regression Fail — config.rs past 1k; main.rs larger god-file
No obvious missed simplification Fail — Workspace splat, dual invert, dual team fetch
No unjustified file-size explosion Fail — config 724→1001
No spaghetti special-case growth Partial fail — default team only on 2 call sites
No wrong-layer / incomplete abstraction Fail — default team not a real CLI-wide default
Behavior for the three issues OK if tests + manual checks pass

Verdict: REQUEST CHANGES — fix the structural items above (especially 1–4), then re-review.

Extract setup into commands/setup.rs, centralize fetch_all_teams and
resolve_team_arg/require_team, add Workspace::with_api_key, simplify
blocked-by display, wire defaults into sprint/statuses/cycles/triage,
and keep config.rs under 1k lines.
@Finesssee
Finesssee merged commit 7c45b4f into master Jul 18, 2026
@Finesssee
Finesssee deleted the fix/setup-config-relations-34-35-36 branch July 18, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants