fleet/fleetd
3
0
Fork 0
Code Issues 78 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

config-edit.sh --set rewrites the whole fleetd.yaml, not the one key — use --from for this file #641

New Issue
Open
opened 2026-10-01 19:05:33 +02:00 by ltms · 0 comments
ltms commented 2026-10-01 19:05:33 +02:00
Owner
Copy Link
Copy Source

Found on config-edit.sh's first live use (#618 item 3), by running --dry-run before
applying. Nothing was broken — the dry-run is what caught it.

--set is not a surgical edit. apply_set_pairs runs yq eval -i on a copy of the live file, and
yq rewrites the entire document in its own output style. On fleetd/fleetd.yaml that reformats
roughly 200 lines to change one value.

What --set .profiles.local.autoCompactWindow=300000 actually does

From the real --dry-run against the live config:

  • Every blank line between top-level blocks is deleted. profiles:, health:, broker:,
    models: and the rest all run together.
  • Every aligned trailing comment is re-indented to column 4. This is the worst part. sonnet's
    maxLoad: 3 carries ~25 lines of aligned commentary recording a measured finding (the
    "only 2 usable" theory and the 2026-08-29 test that disproved it). Aligned under the key, it
    reads as documentation of that key. Flattened to column 4 it reads as unrelated top-level
    commentary. Same for xf's withdrawal note and weight:'s rationale.
  • bootstrapText: >- is folded onto a single line. The value is unchanged (a folded scalar
    joins lines with spaces) but the text a fresh lead is given becomes one unreadable line.
  • Flow style is normalised: argv: [ "claude" ] → argv: ["claude"].
  • Trailing whitespace is stripped (- AI_GATEWAY_TOKEN → - AI_GATEWAY_TOKEN).
  • The value becomes a quoted string: autoCompactWindow: "300000", not 300000. This part is
    already settled as safe in #636 and is not the complaint.

No value changes — measured, with a control

I compared the parsed documents rather than the text, because reasoning about a reformat is not
evidence:

yq -o=json -P 'sort_keys(..)' before.yaml > b.json
yq -o=json -P 'sort_keys(..)' after.yaml  > a.json
diff b.json a.json          # exactly 2 lines: the target key

Exactly 2 differing JSON lines — only autoCompactWindow. I separately sha256'd the five
multi-line values that matter most (the four fleet.charters.* and leadRollover.bootstrapText):
all five unchanged, so charterSha256 is not disturbed and the cross-host comparison still
works. Control: planting a change into charters.dev moved its digest, so the method can detect a
real difference.

So this is a readability and reviewability defect, not a correctness one.

Why it still matters here

fleetd.yaml is not ordinary config. It is an instruction surface whose comments record measured
findings, several of them warnings against repeating a specific mistake. A 200-line reformat also
makes the install diff unreviewable: the one real change is buried, which defeats the point of a
script whose whole purpose is an auditable edit.

The workaround, which works today

build_from_file is a plain cp (scripts/config-edit.sh:456-459), and parse_check only reads.
So --from installs the candidate verbatim — no yq round-trip — while keeping the backup,
parse-check, atomic install and verdict read-back.

That is how I applied #618 item 3: copy the live file, make 4 line-anchored sed edits, confirm
the parsed diff is exactly the 4 intended keys, then --from. Result: exit 3, an install diff of
exactly 4 one-line hunks, and the daemon's own verdict naming
profiles.local/local-direct/opus/sonnet. The file mode survived on both the live file and the
backup.

Asks

  • Document in --help and in the script header that --set reformats the whole file, and that
    --from is the right tool for a file with meaningful comment alignment.
  • Consider making --set warn when its reformat touches more lines than the keys it was asked
    to change — a cheap guard: count changed lines, compare against the number of --set pairs,
    and say so before installing.
  • Decide whether --set should write a bare scalar rather than strenv()'s quoted string.
    #636 settled that the quoted form is safe; it is still a visible difference from every
    hand-written value in the file. Low priority.

Related: #635, #636, #618. Not related to #639 (that one is about redact()).

Found on `config-edit.sh`'s **first live use** (#618 item 3), by running `--dry-run` before applying. Nothing was broken — the dry-run is what caught it. `--set` is not a surgical edit. `apply_set_pairs` runs `yq eval -i` on a copy of the live file, and yq rewrites the **entire document** in its own output style. On `fleetd/fleetd.yaml` that reformats roughly 200 lines to change one value. ## What `--set .profiles.local.autoCompactWindow=300000` actually does From the real `--dry-run` against the live config: - **Every blank line between top-level blocks is deleted.** `profiles:`, `health:`, `broker:`, `models:` and the rest all run together. - **Every aligned trailing comment is re-indented to column 4.** This is the worst part. `sonnet`'s `maxLoad: 3` carries ~25 lines of aligned commentary recording a measured finding (the "only 2 usable" theory and the 2026-08-29 test that disproved it). Aligned under the key, it reads as documentation *of that key*. Flattened to column 4 it reads as unrelated top-level commentary. Same for `xf`'s withdrawal note and `weight:`'s rationale. - **`bootstrapText: >-` is folded onto a single line.** The value is unchanged (a folded scalar joins lines with spaces) but the text a fresh lead is given becomes one unreadable line. - **Flow style is normalised**: `argv: [ "claude" ]` → `argv: ["claude"]`. - **Trailing whitespace is stripped** (`- AI_GATEWAY_TOKEN ` → `- AI_GATEWAY_TOKEN`). - **The value becomes a quoted string**: `autoCompactWindow: "300000"`, not `300000`. This part is already settled as safe in #636 and is not the complaint. ## No value changes — measured, with a control I compared the parsed documents rather than the text, because reasoning about a reformat is not evidence: ```bash yq -o=json -P 'sort_keys(..)' before.yaml > b.json yq -o=json -P 'sort_keys(..)' after.yaml > a.json diff b.json a.json # exactly 2 lines: the target key ``` Exactly **2** differing JSON lines — only `autoCompactWindow`. I separately sha256'd the five multi-line values that matter most (the four `fleet.charters.*` and `leadRollover.bootstrapText`): all five **unchanged**, so `charterSha256` is not disturbed and the cross-host comparison still works. Control: planting a change into `charters.dev` moved its digest, so the method can detect a real difference. **So this is a readability and reviewability defect, not a correctness one.** ## Why it still matters here `fleetd.yaml` is not ordinary config. It is an instruction surface whose comments record measured findings, several of them warnings against repeating a specific mistake. A 200-line reformat also makes the install diff unreviewable: the one real change is buried, which defeats the point of a script whose whole purpose is an auditable edit. ## The workaround, which works today `build_from_file` is a plain `cp` (`scripts/config-edit.sh:456-459`), and `parse_check` only reads. So **`--from` installs the candidate verbatim** — no yq round-trip — while keeping the backup, parse-check, atomic install and verdict read-back. That is how I applied #618 item 3: copy the live file, make 4 line-anchored `sed` edits, confirm the parsed diff is exactly the 4 intended keys, then `--from`. Result: exit 3, an install diff of exactly 4 one-line hunks, and the daemon's own verdict naming `profiles.local/local-direct/opus/sonnet`. The file mode survived on both the live file and the backup. ## Asks - [ ] Document in `--help` and in the script header that `--set` reformats the whole file, and that `--from` is the right tool for a file with meaningful comment alignment. - [ ] Consider making `--set` warn when its reformat touches more lines than the keys it was asked to change — a cheap guard: count changed lines, compare against the number of `--set` pairs, and say so before installing. - [ ] Decide whether `--set` should write a bare scalar rather than `strenv()`'s quoted string. #636 settled that the quoted form is *safe*; it is still a visible difference from every hand-written value in the file. Low priority. Related: #635, #636, #618. Not related to #639 (that one is about `redact()`).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#641
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?