feat(ways): project-scope per-way enable/disable (ADR-131)

[aaronsb]

Summary

• Adds ways disable <name> / ways enable <name> — project-scope toggle for a single way, writes {project}/.claude/ways.yaml.
• New disabled_ways config field, populated only from project overlay (per ADR-131: project scope, no global per-way disabler).
• Enforcement at both gates: Rust ways scan / ways show via session::way_disabled(), bash inject-subagent.sh via an awk parser of the ways: block.
• Default state remains enabled — a project with no overlay behaves identically to today.

ADR

See docs/architecture/system/ADR-131-project-scope-way-toggles.md for context, schema, trust model, and alternatives considered.

Test plan

• cargo test — 106 unit + 11 integration tests pass (8 new disable.rs tests, 7 new config.rs tests including a guard that apply_yaml cannot populate disabled_ways).
• CLI smoke test: ways disable, ways disable --list, ways enable, with multi-way + comment-preservation paths exercised.
• Awk parser smoke test against shorthand + long-form + enabled: true (no-op) + trailing comments.
• (reviewer) Read ADR-131 and confirm the schema choice and project-scope-only constraint are right.
• (reviewer) Sanity-check the YAML editor edge cases in cmd/disable.rs (especially block_is_empty / find_entry indentation handling).

Notes

• The existing disabled_domains user-scope mechanism is untouched — both gates stack (domain-disable OR way-disable → skip).
• The YAML editor preserves comments and unrelated keys by editing only lines inside the ways: block (line-based, not a full re-serialize).

[aaronsb]

Code review — ADR-131

Two blocking correctness defects found end-to-end, plus a few smaller issues. The ADR, doc updates, Rust enforcement at the two gates, and the round-trip tests all look solid; the failure mode is concentrated in the boundary between the line-based YAML editor (cmd/disable.rs) and the awk parser (hooks/ways/inject-subagent.sh), both of which assume a stricter YAML shape than serde_yaml actually accepts.

---

Blocker 1 — Editing a hand-edited 4-space overlay corrupts the file

Where: tools/ways-cli/src/cmd/disable.rs:144-189 (rewrite_block + find_block_end)

find_block_end is hardcoded to base_indent = 0 and the writer is hardcoded to emit {:width$} at base_indent + 2 = 2 spaces. find_entry (line 244) also hardcodes entry_indent = base_indent + 2 = 2. If a user hand-edited their .claude/ways.yaml with 4-space indent (a common YAML style, especially in shops that use 4-space for everything), then:

• ways disable <name> happily appends a 2-space entry below the 4-space children → mixed-indent block → YAML-invalid file.
• ways enable <existing-name> cannot find the 4-space entry (its needle is \" name:\" but the line is \" name:\") → silent no-op even though stdout says "enabled".

Reproduced end-to-end:

```
$ cat .claude/ways.yaml
language: en
ways:
meta/introspection: false

$ ways disable itops/incident
disabled itops/incident (project: …)

$ cat .claude/ways.yaml
language: en
ways:
meta/introspection: false
itops/incident: false ← mixed indent

$ ways config show
[ways] config: parse error: did not find expected key at line 5 column 3 …
language: "auto", ← user's language lost
disabled_ways: [], ← both disabled ways lost
```

The user's thresholds, language, and both disabled ways silently revert to defaults. The CLI never surfaces the parse error — only config show and scan print to stderr (which most scripted paths discard).

Suggested fixes (any one closes the blocker; doing 1+2 is safer):

1. Detect existing indent. In find_block_end, when you find the first non-comment child line, capture its leading whitespace count as block_child_indent. Use that for both find_entry's needle and the writer's emit indent. The block then round-trips at whatever indent the file is already using. This is ~10 lines of code.

2. Pre-flight parse. Before writing, do serde_yaml::from_str(&updated) and refuse to write if it fails — bail with a clear message pointing the user to hand-edit. Cheap (apply_project_ways_overlay already does the parse) and turns a silent corruption into a loud failure.

3. Detect hand-edit and bail with --force. If the existing block's child indent isn't 2, refuse and tell the user to either edit by hand or pass --force (which would then need to renormalize, harder).

The probe that produced these results is on the side; happy to share. Tests that would catch this regression:

```rust
#[test]
fn disable_does_not_corrupt_4space_overlay() {
let input = "ways:\n meta/introspection: false\n";
let out = rewrite_block(input, "itops/incident", true);
serde_yaml::from_str::<serde_yaml::Value>(&out).expect("writer output must be YAML-valid");
}

#[test]
fn enable_removes_4space_entry() {
let input = "ways:\n meta/introspection: false\n";
let out = rewrite_block(input, "meta/introspection", false);
let mut cfg = crate::config::Config::default();
cfg.apply_project_ways_overlay_public(&out);
assert!(cfg.disabled_ways.is_empty(), "enable must actually remove");
}
```

---

Blocker 2 — Awk parser silently misses YAML shapes the Rust parser accepts

Where: hooks/ways/inject-subagent.sh:70-90

The awk parser only recognizes the exact shape the Rust writer emits: 2-space block style, key on its own line for long-form, enabled: false at 4-space indent. The Rust gate uses serde_yaml, which accepts much more. Concrete divergences I reproduced:

Input	Rust gate	Bash gate
ways:\\n foo: false\\n (4-space)	disabled	NOT disabled
ways:\\n foo:\\n enabled: false\\n (4-space long-form)	disabled	NOT disabled
ways: { foo: false } (flow style)	disabled	NOT disabled
ways:\\n foo#x: false\\n	disabled (foo#x)	NOT disabled (awk's sub(/[[:space:]]*#.*$/, …) strips from any #, not just a space-preceded one)

The impact: the same ways.yaml produces different behavior between the main-agent gate (ways scan / ways show) and the subagent gate (inject-subagent.sh). A user disables itops/incident with a 4-space overlay → main agent skips it, every spawned subagent still gets it. This is the exact "subtle bug" you flagged in the review brief.

Suggested fix:

The simplest correctness-preserving approach is to stop parsing YAML in bash and delegate. Two options:

1. Shell out to ways at the top of inject-subagent.sh:
   ```bash
   DISABLED_WAYS=$(ways disable --list --names-only 2>/dev/null)
   ```
   Add a --names-only flag (or repurpose --list to plain-text-emit when stdout isn't a TTY) that emits one name per line, no warnings. Then grep -qxF works as today. This eliminates the bash parser entirely.

2. Inline yq or python -c — but yq is an extra dep and python -c adds latency to every Task call. Option 1 is better since ways is already on the path (the rest of the file assumes it).

If shelling out is unacceptable for cold-start reasons, at minimum:

• Re-detect indent from the first child line instead of hardcoding 2/4 (mirrors fix for Blocker 1).
• Anchor the comment-strip to [[:space:]]+# (space required) so foo#x keeps its #x.
• Document explicitly that flow-style YAML isn't supported and the writer never produces it.

But the divergence problem is structural — any future YAML feature the user picks up from serde_yaml's docs will silently desync the two gates. Delegation is the right shape.

---

Issue 3 — validate_way_name is too permissive

Where: tools/ways-cli/src/cmd/disable.rs:79-90

Inputs that slip through and produce broken-but-silent YAML:

Input	Outcome
\" foo\" (leading space)	writer emits foo: false→ parses as key" foo"`, never matches any real way
\"foo bar\" (internal space)	parses as \"foo bar\", never matches
\"foo#x\"	parses on Rust side, awk strips it → cross-gate divergence (see Blocker 2)
\"foo/./bar\"	parses, never matches a real way
\"foo\\nbar\" after a literal \\n in argv would already be blocked, but a raw newline in shell-passed args isn't always blocked depending on shell quoting

Real way names are constrained to ^[a-z0-9_-]+(/[a-z0-9_-]+)*$ (look at the corpus — they're all lowercase ASCII with - and _, separated by /). I'd suggest:

```rust
fn validate_way_name(name: &str) -> Result<()> {
let valid_char = |c: char| c.is_ascii_alphanumeric() || c == '' || c == '-' || c == '/';
if name.is_empty() { anyhow::bail!("way name cannot be empty"); }
if !name.chars().all(valid_char) {
anyhow::bail!("way name must contain only [a-zA-Z0-9-/]");
}
if name.starts_with('/') || name.ends_with('/') || name.contains("//") {
anyhow::bail!("way name must not start/end with '/' or contain '//'");
}
if name.contains("..") || name.contains("./") {
anyhow::bail!("way name must not contain '..' or './'");
}
Ok(())
}
```

This fails closed on every garbage input above and removes any risk of YAML-special characters (:, #, \", ', \\, &, *, !) reaching the writer.

---

Issue 4 — Clap should express the mutually-exclusive args

Where: tools/ways-cli/src/main.rs:602-614 and Commands::Disable at line 246-252.

Currently:

• ways disable (no args) → eprintln + exit 2. OK.
• ways disable --list itops/incident → silently ignores the positional, runs list(). Empirically confirmed — exit 0, no warning, user thinks the way was disabled and moves on.

Expressing it in clap surfaces the misuse:

```rust
Disable {
/// Way ID to disable (mutually exclusive with --list)
#[arg(required_unless_present = "list", conflicts_with = "list")]
name: Option,
/// List currently disabled ways in this project
#[arg(long)]
list: bool,
},
```

Then ways disable --list itops/incident produces clap's standard "the argument '--list' cannot be used with '[name]'" error with exit 2 — same exit code, but the user sees why. Worth the change because the silent-ignore failure mode here compounds with Blocker 1 (both are "CLI reports success, nothing happened").

---

Issue 5 — Project-scope invariant is structurally fragile

Where: tools/ways-cli/src/config.rs:38, 92-115

The invariant "only apply_project_ways_overlay populates disabled_ways" is currently enforced only by the fact that apply_yaml doesn't read the ways: key. The test apply_yaml_does_not_populate_disabled_ways (line 411) is a good guard but ADR-131 explicitly contemplates future per-way threshold overrides reading the same ways: block — at which point someone may innocently extend apply_yaml to read ways: and break the invariant.

disabled_ways: Vec<String> is pub, so any code in the crate can push to it without going through the overlay parser. Not a blocker (you have the guard test), but worth noting that the ADR's structural promise ("project scope only") is enforced by convention, not by type.

Possible hardening (optional, not blocking):

• Make disabled_ways private with a single setter populate_disabled_ways_from_project_overlay(&str) that's the only code path that can write it.
• Or: track a disabled_ways_source: Option<PathBuf> next to the field, and panic in debug if it's ever set from a non-project layer. Loud failures during tests would prove the invariant.

---

What looks solid

• ADR-131 itself is exemplary — explicit alternatives, trust-model reasoning, forward-compat note for long-form schema.
• apply_project_ways_overlay (config.rs:198-221) is clean — handles both shorthand and long-form, dedupes correctly, ignores future-reserved subkeys per the schema's intent. The enabled: true no-op test (line 370) is the right kind of negative test.
• Enforcement at the Rust gates (scan/candidates.rs:71, show/mod.rs:23 and :173) is two-line, symmetric, and matches the existing domain_disabled pattern — easy to audit.
• session::way_disabled (line 445) is a one-liner mirroring domain_disabled. Consistent shape.
• Doc updates in extending.md / itops.md / meta.md correctly position project-scope-per-way against the existing global-domain disable, with the right "never anywhere vs. not in this project" framing.
• cmd/disable.rs overall structure — helpers are well-named and single-purpose (validate_way_name, way_exists, read_or_empty, is_disabled, rewrite_block, find_block_end, find_entry, block_is_empty). Three public entry points. 385 lines total with ~95 of tests is fine; not overengineered.
• is_disabled routes through the real apply_project_ways_overlay_public so writer and reader can never disagree on what the writer wrote. Excellent self-consistency check.

---

Suggested merge order

1. Blockers 1 + 2 — both can be addressed with the same fix (detect indent from existing block + delegate bash to ways). Add the YAML-validity assertion as a regression test.
2. Issue 3 (validation tightening) — small diff.
3. Issue 4 (clap) — small diff, big UX win.
4. Issue 5 (invariant hardening) — optional, file a follow-up issue if you'd rather land the ADR-131 surface first.

Findings reproduced from a local probe against the branch — happy to attach the probe source if useful.

[aaronsb]

Follow-up review on f206325

All blockers and follow-ups verified fixed. Mergeable.

B1 — 4-space overlay corruption: fixed

• find_block_end (tools/ways-cli/src/cmd/disable.rs:244-268) now reads child indent from the first existing entry; rewrite_block (line 210) reuses that width.
• write_overlay (line 144) pre-flights through serde_yaml::from_str and refuses to write on parse failure.
• End-to-end repro: hand-edited 4-space shorthand AND long-form both round-trip through disable then enable cleanly. Final file parses via python3 yaml.safe_load. Indent stays 4-space — no mixed-indent output.

B2 — Awk parser divergence: fixed

• Awk YAML parser removed from hooks/ways/inject-subagent.sh; replaced with ways disable --list --names-only at line 66. The two remaining awk calls (lines 109, 132) are frontmatter parsers on individual way .md files, unrelated.
• --names-only stdout is bare-name only — verified no ? markers, no trailing commentary, clean stderr.
• All four divergence cases from the original review now surface through --names-only:
  • 4-space shorthand → itops/incident
  • 4-space long-form → itops/incident
  • flow style ways: {a: false, b: false} → both listed
  • quoted keys with # → emitted intact

I3 — validate_way_name: fixed

• Charset-anchored to [a-z0-9_-]+(/[a-z0-9_-]+)*$ per segment (cmd/disable.rs:103).
• Test list at cmd/disable.rs:344-366 covers every slip-through I flagged (space, uppercase, #, \", \\, :, dot-segments, leading/trailing/double slash, newline). Manually verified each via CLI — all reject with clean error.

I4 — clap conflicts: fixed

• ways disable --list itops/incident exits 2 with error: the argument '--list' cannot be used with '[NAME]'. No silent list-run.
• --names-only without --list also fails clean via requires = \"list\" (main.rs:255).

I5 — disabled_ways visibility: fixed and structural

• Field is pub(crate) (config.rs:45); external readers use disabled_ways() accessor at config.rs:99.
• Crate-internal writers: only apply_project_ways_overlay (line 229-230) mutates it outside Default.
• apply_yaml does not touch the field, and the test apply_yaml_does_not_populate_disabled_ways (config.rs:423) locks this in — any future regression in the wrong layer fails CI loudly. Invariant is structural, not conventional.

Test results

cargo test --release clean: 112 unit + 11 sim tests passing, including 4 new disable tests targeting the exact B1 scenarios.

LGTM for merge.

---

AI-assisted follow-up review via Claude