Skip to content

Make wt config show a real diagnostic - #3999

Open
max-sixty wants to merge 28 commits into
mainfrom
land/config-command
Open

Make wt config show a real diagnostic#3999
max-sixty wants to merge 28 commits into
mainfrom
land/config-command

Conversation

@max-sixty

@max-sixty max-sixty commented Sep 2, 2026

Copy link
Copy Markdown
Owner

wt config show reported a broken config and exited 0, so nothing scripted could branch on it, and several of its claims were wrong. This makes it a real diagnostic and corrects the config page it points at.

wt config show

  • Exits non-zero when configuration or approval state is invalid. The text report still renders in full; JSON remains parseable and uses null for a source it could not deserialize. Warnings (unknown keys, deprecations) leave the exit at 0.
  • Validates [list] columns against the resolved config, with the message wt list uses. Column names resolve in the command layer, so a typo used to surface only when wt list aborted on it.
  • Reports approval state in its own APPROVALS section. An unreadable user-level approvals file is invalid in every repository; valid state names project commands still awaiting approval. The config dump reads as "these run"; on a fresh clone, hooks stop at the approval prompt the first time one fires.
  • Renders SYSTEM CONFIG unconditionally, naming the default path when no file exists. The help text said "Also shows system config if present" while the output reported its absence.
  • A misplaced key whose home is a [projects."…"] entry got a hint carrying the literal placeholder — add it under [projects."<id>"] in user config — leaving the reader to derive an identifier the report prints two lines above. The hint now names the real one, resolved once at the top and threaded through, so the Identifier: line and the hint cannot disagree.
  • A new EFFECTIVE section gives the value every scalar setting resolves to once --config-set, WORKTRUNK_*, the matching [projects] entries, the global keys and system config have applied. The file sections show what each layer contains; reading a value off them meant replaying the merge by hand. Every scalar is listed, not only those differing from a default: a differs-only list answers "what is in force" only for a reader who already knows the defaults, and usually renders empty.

wt config create / update

  • create wrote a file that warned about [list] json-schema the first time anything read it — every line of the template is a comment, so the pending default it documents was never set. It now routes the template through compute_migrated_content, emitting exactly what wt config update would write.
  • wt config update --output=- > config.toml keeps the migrated TOML and could silently lose every [projects."…"] approved-commands array: the in-place path copies them to approvals.toml first, but output mode writes no approvals file. It now names the affected entries on stderr, with the command that migrates them, leaving stdout pipeable.

The config page

Claims the source contradicts
  • [list] full bundles the CI column, and the Summary column only when [list] summary and a [commit.generation] command are both set.
  • [list] timeout-ms is discarded under --full.
  • columns = ["branch", "status", "ci", "path"] and json-schema = 2 sat under "values shown are defaults". Neither is a default, and the real default column set carries no ci.
  • Env overrides reach scalars only — arrays and maps carry one string per key, so those go through --config-set.
  • XDG_CONFIG_DIRS is Unix-only, and replaces a platform default list that differs on macOS.
  • [projects."…"] approved-commands was documented as live; it is deprecated. Its pattern rule moves to wt config approvals, which now shows what approvals.toml actually looks like.
  • {{ repo }} is also spelled {{ main_worktree }} — one value, two names, one documented.
  • wt step copy-ignored --from defaulted to "main worktree"; the code resolves the primary worktree, which for a bare repo is the default-branch worktree.
Structure and undocumented surface
  • ### List, ### Commit, … sat at H3 under a ## Command config wrapper, and the site table of contents caps at H2, so none were reachable from the sidebar. The wrapper is gone and its children move up a level; heading text is unchanged, so every /config/#… anchor still resolves.
  • shell, update, and plugins join the subdoc markers, which is what puts wt config update — named in the deprecation warnings — and wt config shell show-theme on the page. create stays out, with a comment saying why: its after_long_help is the page body, so a subdoc would nest the page inside itself.
  • A per-key "which file" table near the top.
  • [post-start] copy = "wt step copy-ignored" in both copy-ignored sections, and so in the project template.
  • WORKTRUNK_APPROVALS_PATH beside its two siblings; skip-shell-integration-prompt and skip-commit-generation-prompt; --config <path>, which had appeared only inside generated help blocks, with its place in Precedence.
  • Two USER_CONFIG links were bare same-page fragments. That block renders three ways — the page, terminal help, and dev/config.example.toml — so outside the page they landed as Custom prompt templates (#custom-prompt-templates), pointing at nothing. Written root-relative, the sync pipeline expands them for the portable copies.
  • The five [commit.generation] tables are alternatives; uncommenting the section wholesale is a TOML duplicate-key error, which the lead now says.
  • The worktrunk skill pointed at the removed #command-config anchor. The writing-user-outputs skill told the model to suggest wt config validate, which does not exist — wt config show is the command that reports what is wrong with a config.

UX survey items: #14, #15, #25, #30, #31, #42, #43, #44, #59, #60.

Reviewable files: src/commands/config/{mod,show,update,create}.rs, src/cli/config.rs, the USER_CONFIG / PROJECT_CONFIG blocks in src/cli/mod.rs, skills/worktrunk/SKILL.md, tests/integration_tests/config_show.rs. Generated mirrors and snapshots are regenerated.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb

This was written by Codex on behalf of max-sixty

max-sixty and others added 10 commits September 1, 2026 16:33
`config show` reported a broken config and exited 0, so nothing scripted
could branch on it, and three of its claims were wrong.

- Exit non-zero when a config is invalid. The whole report still renders;
  `AlreadyDisplayed` carries only the code. Warnings (unknown keys,
  deprecations) leave the exit at 0.
- Validate `[list] columns` against the resolved config, with the message
  `wt list` uses. The names resolve in the command layer, so a typo used to
  surface only when `wt list` aborted on it.
- Report project commands still awaiting approval under PROJECT CONFIG. The
  config dump reads as "these run", but a fresh clone's hooks stop at the
  approval prompt the first time one fires.
- Render SYSTEM CONFIG unconditionally, naming the default path when no file
  exists, instead of folding its absence into a hint under USER CONFIG. The
  help text said "Also shows system config if present" while the output
  reported its absence.
- Replace "If a config file doesn't exist, shows defaults that would be used"
  — it printed no defaults — with what the command actually reports.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`wt config update --print > config.toml` kept the migrated TOML and lost
every `[projects."…"] approved-commands` array it named: the write path
copies them to `approvals.toml` before rewriting, but `--print` writes no
file, and the dropped keys never appear in the printed output.

`--print` now names the affected `[projects]` entries on stderr, with the
command that migrates them, leaving stdout pipeable. `Approvals::
load_from_config_file` becomes public so the warning can list them.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`wt config create` wrote a file that warned about `[list] json-schema` the
first time anything read it: every line of the template is a comment, so the
pending default it documents was never actually set. `create` now routes the
commented template through `compute_migrated_content`, emitting exactly what
`wt config update` would write, from the table that owns those defaults.

`wt config show` gains an `EFFECTIVE` section: the value every scalar setting
resolves to once `--config-set`, `WORKTRUNK_*`, the matching `[projects]`
entries, the global keys and system config have applied. The file sections
show what each layer contains; reading a value off them meant replaying the
merge by hand.

Every scalar is listed rather than only those differing from a default — a
diagnostic is read to find out what is in force, and a differs-only list
answers that only for a reader who already knows the defaults, while usually
rendering an empty section. Arrays and tables accumulate across layers rather
than replacing, so the file dumps already show every contribution and the
section leaves them out.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…44, #59, #60)

Claims the source contradicts (#59, #60):

- `[list] full` bundles the CI column, and the Summary column only when
  `[list] summary` and a `[commit.generation]` command are both set.
- `[list] timeout-ms` is discarded under `--full`.
- `columns = ["branch", "status", "ci", "path"]` and `json-schema = 2` sat
  under "values shown are defaults"; neither is a default, and the real
  default column set carries no `ci`. That sentence is now scoped to the
  blocks it holds for.
- Env overrides reach scalars only — arrays and maps carry a single string
  per key, so those go through `--config-set`.
- `XDG_CONFIG_DIRS` is Unix-only, and replaces a platform default list that
  differs on macOS.
- `[projects."…"] approved-commands` was documented as live; it is
  deprecated. Its pattern rule moves to `wt config approvals`, which now
  shows what `approvals.toml` actually looks like.

Undocumented surface (#60): `WORKTRUNK_APPROVALS_PATH` beside its two
siblings; `skip-shell-integration-prompt` and `skip-commit-generation-prompt`,
which now reach the generated template; `--config <path>`, which had appeared
only inside generated help blocks, with its place in Precedence.

Structure:

- #31 — `### List`, `### Commit`, … sat at H3 under a `## Command config`
  wrapper, and the site ToC caps at H2, so none were reachable from the
  sidebar. The wrapper is gone and its children move up a level; heading text
  is unchanged, so every `/config/#…` anchor still resolves.
- #42 — `shell`, `update`, and `plugins` join the subdoc markers, which is
  what puts `wt config update` (named in the deprecation warnings) and
  `wt config shell show-theme` on the page.
- #43 — a per-key "which file" table near the top, and the `url` column row
  now says the template comes from project config.
- #44 — `[post-start] copy = "wt step copy-ignored"` in both copy-ignored
  sections, and so in the project template.
- #25 — the in-repo `worktree-path` example says to ignore the directory.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A misplaced key whose home is a `[projects."…"]` entry gets a note saying
where to put it, and the note carried the literal placeholder: `add it under
[projects."<id>"] in user config`. The reader then had to derive the
identifier — which `config show` prints two lines above, under PROJECT
CONFIG, precisely because deriving it from the remote URL is the hard part.

`config show` now passes that identifier down, so the suggested key is one
the reader can paste. Config load has no repository in hand and keeps the
placeholder. The identifier is resolved once at the top of the report and
threaded through the sections, so the `Identifier:` line and the hint cannot
disagree.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BGS51k25qzfeJEJ8dfxEEY
#60)

The `USER_CONFIG` block is rendered three ways — the config page, terminal
help, and `dev/config.example.toml`, which `wt config create` writes — and
two of its links were written as bare same-page fragments. On the page they
resolved; everywhere else they landed as `Custom prompt templates
(#custom-prompt-templates)` and `custom columns (#custom-columns)`, pointing
at nothing (#30). Written root-relative, the sync pipeline expands them to
`https://worktrunk.dev/config/#…` for the portable copies and the page keeps
its anchor.

Also from the same block (#30): five `[commit.generation]` tables sit under
five `###` headings, one per CLI, and uncommenting the section wholesale is a
TOML duplicate-key error. The lead now says they are alternatives.

#60: `{{ repo }}` is also spelled `{{ main_worktree }}` — one value, two
names, and only one of them documented. `wt step copy-ignored --from`
defaulted to "main worktree" while the code resolves the *primary* worktree,
which for a bare repo is the default-branch worktree instead.

#42: `create` is the one `wt config` subcommand still without a subdoc
marker, and a comment beside it now says why — its `after_long_help` is the
config page's own body, so a subdoc would nest the page inside itself.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BGS51k25qzfeJEJ8dfxEEY
Promoting `### List`, `### Commit`, … out from under `## Command config`
(#31) removed the `#command-config` anchor, and the worktrunk skill's
"Configure command defaults" entry still pointed at it. It now names the
sections that replaced it and links the first.

The `writing-user-outputs` skill's model diagnostic message ends `run wt
config validate for details`. There is no `wt config validate`; the command
that reports what is wrong with a config is `wt config show` — the same
command the skill's own warning-placement table routes these issues to
(#14).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BGS51k25qzfeJEJ8dfxEEY
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
The typos hook's `unparseable` → `unparsable` fix landed in the source without
the insta snapshot that renders it, so `help_config_show` failed. Regenerated,
along with the config mirrors.

While there: the sentence wrapped mid-code-span, so the renderer closed the dim
run at the line break and reopened it over the following clause — the terminal
showed `[list]` dimmed, then `columns name no column answers to, and project
commands still awaiting` dimmed, which is not the span anyone wrote. Rewrapped
so `[list] columns` stays on one line.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three accuracy points inline. Nothing structural — the diagnostic itself holds together: the exit-code contract is consistent across the three file sections (config_show_invalid_{user,project,system}_* all pin exit_code: 1, warnings-only pins 0), threading the resolved identifier through with_scope_note keeps the Identifier: line and the misplaced-key hint from disagreeing, and format_dropped_approvals_warning is correctly gated on DeprecationKind::ApprovedCommands with the entries read back from the candidate's own file rather than from the merged config.

One asymmetry not worth its own thread: in render_user_config the check_and_migrate Err arm sets invalid = true, but the identical arm in render_project_config doesn't. A parse failure is caught either way by the toml::from_str::<ProjectConfig> below, so nothing observable changes today — it only means the two sections would answer "is this sound?" differently if check_and_migrate ever failed for a non-parse reason.

Reviewed at 25e1907; the wording/snapshot fix in that commit clears the help_config_show mismatch that was failing test (linux), test (macos), and code-coverage on the previous head.

Comment thread src/cli/mod.rs Outdated
Comment thread src/commands/config/show.rs Outdated
Comment thread src/commands/config/show.rs Outdated
…orktree

`render_column_selection` returned `Ok(false)` when `resolve_custom_columns`
failed, so a zero width, a blank header, an unknown template variable, or more
than 256 entries left `wt config show` green while `wt list` aborted on it —
the exact split the `columns` check exists to close. The resolve now reports
its own failure. It also runs before the empty-selection early return, because
custom columns append to the default set: a broken definition aborts `wt list`
whether or not `[list] columns` names it, and the default case sets no
selection at all.

"Every scalar is listed" wasn't true — the three `[commit.generation]` prompt
templates and the two first-run prompt flags are absent — and an absent row
read as "nothing sets this" is the misreading the differs-only design was
rejected to avoid. Both the docstring and the help text now say what the
section covers and why those five are out.

`{{ main_worktree }}` is the deprecated old name for `{{ repo }}`, not a second
spelling: it is in `DEPRECATED_TEMPLATE_VARS`, so following that line earns a
deprecation warning on every load. Dropped from the worktree-path variable
list.

Also set `invalid` in the project-config `check_and_migrate` error arm, which
its user-config twin already did.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
@max-sixty

Copy link
Copy Markdown
Owner Author

All three applied, plus the invalid asymmetry.

The custom-columns one was the substantive catch: reproduced your width = 0 config, which gave a green config show and a failing wt list. Resolving now reports its own failure, and it runs before the empty-selection early return — your second point, and the one that matters more, since custom columns append to the default set and the default case sets no selection at all. Same config now: ✗ Invalid [list.custom-columns.Ticket] width: must be at least 1, exit 1.

On the EFFECTIVE claim I took the narrow-it option for all five rather than adding rows: the templates are multi-line strings that would swamp a one-line-per-row section, and the two prompt flags record whether a prompt was answered rather than configuring anything a reader is diagnosing. Stated in both the docstring and the help text, as you said it had to be.

main_worktree is gone — a line steering readers into a deprecation warning is exactly what this branch is supposed to be removing.

This was written by Claude Code on behalf of max-sixty

@worktrunk-bot

Copy link
Copy Markdown
Collaborator

All three landed as described — and running the custom columns before the empty-selection return is the right call: with no columns set at all the old ordering checked nothing, which is the common case.

Two things still open, one of them red CI.

help_config_create is a stale snapshot. code-coverage failed on it at 13956e4, and test (linux), test (macos), test (windows) are still pending on the same assertion. Dropping main_worktree from the USER_CONFIG block regenerated help_config_long and help_config_show, but help_config_create.snap:95 still carries the dropped also spelled {{ main_worktree }} tail. cargo insta test --accept --test integration -- test_help clears it — that snapshot is the only file left in the tree matching also spelled.

The EFFECTIVE narrowing isn't in front of the reader who sees the section. The docstring and wt config show --help both say it now, but the rendered output is unchanged: EFFECTIVE + Resolved scalar settings, every layer applied, the unqualified claim. So the misreading survives in the place it was raised about — [commit.generation] renders as # command unset and nothing else, and a reader takes that for "no template set" rather than "not covered here". Relatedly, show.rs:1032 now asserts "The section's own heading says which settings it covers" — format_heading("EFFECTIVE", None) doesn't, so that's a fresh instance of the claim-the-source-contradicts shape this branch is clearing. Scoping the info_message line is the version a config show reader actually gets.

Smaller: the new resolve-failure arm (show.rs:774-777) has no test — nothing under tests/integration_tests/config_show.rs configures [list.custom-columns] at all, so codecov/patch will see those lines as a miss. test_config_show_unknown_list_column (config_show.rs:1289) is the shape to mirror with the width = 0 config from the review.

Evidence
  • code-coverage failure at 13956e4: run 33706174941snapshot assertion for 'help_config_create' failed in line 55, diff is the single {{ repo }} line, losing the also spelled {{ main_worktree }} tail. affected tests (linux, advisory) failed in the same window; its log wasn't retrievable yet (run still in progress), so I haven't confirmed it's the same assertion.
  • git grep -l 'also spelled' over the tree at 13956e4 returns exactly one path, the snapshot above — the source, dev/config.example.toml, and the three doc mirrors are all updated.
  • [commit.generation] in the rendered section, from config_show_with_project_config.snap: the table contains # command unset and nothing else; template, squash-template, template-append have no rows.
  • resolve_custom_columns in src/commands/list/custom_columns.rs runs no subprocess and returns immediately on an empty map, so hoisting it above the early return costs nothing when [list.custom-columns] is unset.

The `main_worktree` removal reaches `wt config create --help` through
`dev/config.example.toml`, which regenerates from the same USER_CONFIG block —
so the snapshot had to be taken after that file was rewritten, not before.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the snapshot regeneration in 9db35a1: cargo test --test integration help:: is 56/56 on the merged tree, so the red on test (linux), test (macos), and code-coverage clears at this head.

One point inline that hasn't come up yet — the reach of the new exit-code contract.

Comment thread src/commands/config/show.rs
max-sixty and others added 2 commits September 2, 2026 19:30
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
Adding the `update` subdoc marker is what puts `wt config update`'s
`after_long_help` on the config page, so #3994's removal clause reaches the
mirrors here rather than there.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XAUYWFN9d9oh6jyoQiouHb
max-sixty pushed a commit that referenced this pull request Sep 4, 2026
…example (#4006)

## Problem

The `worktree-path` examples in the user-config guide are introduced as
being **"for repo at `~/code/myproject`"**, and every example states the
path it produces. For all but one, that arithmetic checks out. The
bare-repository example doesn't:

```toml
worktree-path = "{{ repo_path }}/../{{ branch | sanitize }}"
```

Heading claimed `~/code/myproject/feature-auth`. With `repo_path` at
`~/code/myproject` as the section says, `{{ repo_path
}}/../feature-auth` resolves to `~/code/feature-auth` — one directory up
from the stated result.

## Fix

The template is right; the heading silently switched the repo's location
without saying so. `{{ repo_path }}` for a bare repo is the bare
directory itself (as the variable list a few lines above states), so the
claimed result holds only when that directory is a hidden child — the
`myproject/.git` layout that [tips-patterns.md
documents](https://github.com/max-sixty/worktrunk/blob/main/docs/src/content/docs/tips-patterns.md#bare-repository-layout),
and that `wt switch`'s bare-repo offer writes this exact template for.
So the heading now names it:

> Bare repository cloned to `~/code/myproject/.git`
(`~/code/myproject/feature-auth`):

Edited in `src/cli/mod.rs` (the primary source); the four generated
mirrors and two `--help` snapshots are regenerated output.

## Testing

No regression test — this is a documentation string with no behavior
attached. The generated mirrors are pinned by the existing sync tests,
which is what caught them here:

- `cargo test --test integration readme_sync` — 18 passed (regenerates
`dev/config.example.toml`, `docs/src/content/docs/config.md`, and both
`skills/.../reference/config.md` mirrors).
- `cargo insta test --accept --test integration -- test_help` — 47
passed (`help_config_create`, `help_config_long`).
- `cargo fmt --check` — clean.
- `cargo test --test integration` — 2048 passed, 1 failed.

<details><summary>The one integration failure is a sandbox artifact, not
a regression</summary>


`step_copy_ignored::test_copy_ignored_preserves_file_executable_permissions`
expects `0644` and gets `0664`. The tend sandbox runs with `umask 0002`
(group-writable) rather than the `0022` the test assumes.

Confirmed unrelated: it reproduces identically with this branch's
changes stashed, i.e. on the merge base. Both `ci` and `coverage` on
`main` are green at `2026-09-03T10:17:26Z`. This diff touches only doc
strings and snapshot files and cannot reach file-permission code.

</details>

<details><summary>Checked against the in-flight docs PRs</summary>

#4000, #3999, and #3998 each touch the same five files (`src/cli/mod.rs`
plus the four config mirrors), so I checked for the duplication that
sank #4001. None of them edits the `worktree-path` examples region, and
`git merge-tree` against each reports a clean merge with this branch.

</details>

Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com>

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_docs_are_in_sync clears at f7905a15 — I reran it against the merged tree and it passes with nothing rewritten, so the three config.md mirrors now carry the pi entry b8f59e2a6 merged in.

Two inline on 5f19c507b. The first is the one that matters: --format json now carries the exit-code contract, but derives it from the column check alone, so it still answers 0 for a config the text form calls invalid.

Comment thread src/commands/config/show.rs Outdated
Comment thread src/commands/config/show.rs Outdated

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things on 3caf96ad7. The tests themselves check out — I ran all three against the merged tree and they pass, and the ansi_strip on the dropped-approvals assertion is the right fix, since <bold> sits between 2 and [projects] in that warning.

The eight writeln! reflows are coverage-shaped, not readability-shaped. Each one lands on a line codecov/patch flagged as a miss at f7905a15 — in that commit's numbering, show.rs 660, 1098, 1108, 1115, 1129, 1137, 1158, 1214, 1221, every one of them the closing )?; of a multi-line macro — while the other 40 multi-line writeln!( in the file are untouched, including the one on the very next line at show.rs:656. That the only miss in an eight-line macro was the )?; is the tell that the block was reached: the miss isn't the write, it's the ?. These renderers write into a String through fmt::Write, which cannot return Err, so llvm-cov records an early-return region on the )?; line that no input can ever hit. Collapsing the macro onto one line moves that region next to covered code and the line reports green — the number moves, the untaken branch doesn't.

CLAUDE.md → Coverage: "neither the code nor the tests get contorted to move the number." The codebase already carries the form that removes the region instead of hiding it — let _ = writeln!(…), 42 sites in src/, including format_dropped_approvals_warning in this same PR (update.rs:159). Applying that to the renderers is durable in a way the reflow isn't: as landed, the next multi-line writeln! added to render_effective_config brings the miss straight back.

Smaller, same commit: test_config_show_without_project_identifier_uses_global_worktree_path executes the None arm at show.rs:1085 but cannot fail on it. The test config has no [projects] entry, so worktree_path_for_project(id) and worktree_path() return the same string and the assertion holds whichever arm runs; likewise !stdout.contains("awaiting approval") is satisfied by the load_project_config() guard two lines further down, since the test writes no project config. It covers the lines without defending the behavior.

The other point is inline.

Comment thread src/commands/config/show.rs Outdated

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things on f4f51b0af, both about how far the new checks reach rather than whether they work. I built the merged tree and ran the five changed/added tests — all pass — plus the reported case: no user config file, unparsable approvals.toml, wt config show and --format=json now both exit 1. Dropping the empty-selection guard in validate_column_selection checks out too: a width = 0 custom column with no [list] columns set reports and exits 1 in both formats, and a valid one still exits 0 in both.

Comment thread src/commands/config/show.rs
Comment thread src/commands/config/show.rs

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three things on f4f51b0af..22f7cd23a, all the same shape the earlier threads here were about — the JSON surface and the text one answering differently — but on the read path rather than the parse path. I built the merged tree and reproduced each against target/debug/wt; commands and output in the details block.

--format=json exits 0 for a config file wt cannot read. 2bc28f612 added the read-failure arm for the system file (Err(_) => true on std::fs::read_to_string), but the user file got no equivalent. UserConfig::load_with_warnings skips a layer whose read fails and records no warning — the if let Some(system_path) = path::system_config_path() && let Ok(content) = std::fs::read_to_string(&system_path) shape in src/config/user/mod.rs, repeated for the user layer — so UserConfig::load() returns Ok and nothing sets invalid. With the user config at mode 000, wt config show --format=json exits 0 and reports "config": {"projects":{}}; wt config show exits 1 on the same state. That is the health check this PR exists for, green on a config the text form calls broken. Inline, with the minimal arm.

A malformed system config nulls user.config. UserConfig::load() merges all three layers and surfaces the first warning from any of them, so the Err(_) arm in the user branch fires for a failure that isn't the user file's. Same user file, exit 1 either way, but user.config is null with a malformed system config and {"list":{"json-schema":2},"projects":{}} without one. The text report gets this right because it is per-file — it renders ✗ Invalid config under SYSTEM CONFIG and dumps the user config in full underneath. test_config_show_json_rejects_invalid_system_config now constructs exactly this state (the empty user config added in e62777469 plus the malformed system file) and asserts only on system.exists, so the misattribution is pinned-adjacent but unexamined. The root is shared with the point above: the JSON path derives each source's verdict from a whole-stack loader, while the text renderers derive each from that file's own contents. Mirroring the text model fixes both at once — though what user.config should hold when a lower layer is the broken one is a call I'd leave to you, which is why the inline suggestion only covers the first half.

The text report is the one that no longer renders in full. handle_config_show's docstring is where the contract is written — "the whole report still renders, and the non-zero exit comes after it" — but all three text renderers still propagate an I/O failure and abort it: render_system_config's std::fs::read_to_string(&system_path).context("Failed to read system config file")?, and the .context("Failed to read config file")? in render_user_config and render_project_config. Against the same state the new test_config_show_json_rejects_unreadable_system_config pins, JSON prints a 401-byte report and exits 1 while text prints 0 bytes of report and a bare ✗ Failed to read system config file. A .config/wt.toml at mode 000 does the same thing. Before this PR both surfaces bailed together, so this is a divergence the incremental opened rather than one it inherited — and it lands on the surface the contract paragraph is written about.

Smaller, and the PR's own genre: the help text still says "Every section names its path whether or not the file exists" and files the approvals diagnostics under "Alongside each file". Since e62777469 neither holds for APPROVALS — it is its own section, it names its own path, and it appears only when there is something to report. One sentence in src/cli/config.rs, plus the four generated mirrors.

Unrelated to correctness: #4021 replaces --print with --output and its head has no format_dropped_approvals_warning, so whichever of the two lands second needs to carry the dropped-approvals warning onto the new flag — it is the kind of thing a clean auto-merge drops silently.

Evidence

Built 22f7cd23a (cargo build --bin wt) and ran against a scratch repo with HOME, XDG_CONFIG_HOME, WORKTRUNK_CONFIG_PATH and WORKTRUNK_SYSTEM_CONFIG_PATH pointed into a tempdir. Not root (id -u → 30009), so the mode bits bite.

### F1: user config exists, unreadable (chmod 000), no system config
  json exit=0  user.config={"projects":{}}
  text exit=1  stderr=✗ Failed to read config file

### F2: user config valid, system config malformed
  json exit=1  user.config=null
  (same user file, no system cfg) user.config={"list":{"json-schema":2},"projects":{}}

### F3: system config path unreadable — text report size
  json exit=1 stdout=401 bytes
  text exit=1 stdout=0 bytes  stderr=✗ Failed to read system config file

A .config/wt.toml at mode 000 reproduces F3 on the project renderer too: json exit=1 with {"config":null,"exists":true,…}, text exit=1 with 0 bytes of report.

Checked and cleared while going through approvals_diagnostic: the Err(_) if approvals_file_exists arm cannot misattribute a broken user config to approvals, because Approvals::load_with_fallback reads config.toml only when the approvals file is absent (src/config/approvals.rs, "If the approvals file exists → load from it (authoritative)"), so the two error sources can't cross. The comment on the Err(_) => Valid arm is accurate. default_branch_project_config_content() being reached more often in the JSON path costs nothing either — it returns None before any subprocess for a non-bare repo.

Comment thread src/commands/config/show.rs Outdated
# Conflicts:
#	docs/src/content/docs/config.md
#	docs/tests/built-site.test.mjs
#	plugins/worktrunk/skills/worktrunk/reference/config.md
#	skills/worktrunk/reference/config.md
#	src/commands/config/update.rs
#	tests/integration_tests/config_show.rs

@worktrunk-bot worktrunk-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 22f7cd23a..b6dc824942a269f670 plus the origin/main merge and its two follow-ups.

The user-side half of 2a269f670 is the fix, and it is load-bearing: I deleted just that arm and rebuilt, and --format=json went back to exit 0 with user.config = {"projects":{}} for a user config holding a single 0xff byte. The project-side half is the one point, inline.

The --print/--output collision I flagged last time landed cleanly. The merge resolution moved format_dropped_approvals_warning into write_migrated_output, so it now fires for a file destination as well as stdout, and the regression followed it — test_config_update_output_warns_about_dropped_approvals exercises both --output=- and --output=migrated.toml. Nothing dropped.

Ran on the merged tree at b6dc82494: help::, config_show:: and test_docs_are_in_sync, 218 passing — so the refreshed help_config_update snapshot matches its source and the generated mirrors are in sync. I did not verify the 4746 config-TOC count in docs/tests/built-site.test.mjs; that one is check-docs, which runs on this head.

Comment on lines +171 to +186
let on_disk_unreadable = on_disk
.as_ref()
.filter(|path| path.exists())
.is_some_and(|path| std::fs::read_to_string(path).is_err());
let config = if on_disk_unreadable {
invalid = true;
None
} else {
match repo.load_project_config() {
Ok(config) => config,
Err(_) => {
invalid = true;
None
}
}
};

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProjectConfig::load already fails on this. For an on-disk path that exists it does the same read_to_string and maps the error to ConfigError("Failed to read …"), so load_project_config() returns Err and the arm this wraps already sets invalid and nulls the config. The user layer needed its check because UserConfig::load_with_warnings skips a layer it cannot read and records no warning; the project loader has no such hole, so the two sides aren't symmetric.

Checked by deleting exactly the lines below and rebuilding at b6dc82494: wt config show --format=json against a .config/wt.toml holding one 0xff byte gave byte-identical stdout and exit 1 (same for mode 000), and all 160 config_show:: integration tests passed — test_config_show_json_rejects_unreadable_project_config among them. That test pins the behaviour, but it passes with the code it was added for removed, so what it defends is the loader's own arm rather than this one.

Suggested change
let on_disk_unreadable = on_disk
.as_ref()
.filter(|path| path.exists())
.is_some_and(|path| std::fs::read_to_string(path).is_err());
let config = if on_disk_unreadable {
invalid = true;
None
} else {
match repo.load_project_config() {
Ok(config) => config,
Err(_) => {
invalid = true;
None
}
}
};
let config = match repo.load_project_config() {
Ok(config) => config,
Err(_) => {
invalid = true;
None
}
};

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.

2 participants