Rename the drain to autofix - #60
Conversation
"Drain" was doing three unrelated jobs — the installer, the per-repository permission to write code, and the fix-before-review invariant — while the same word, correctly, also meant "consume the shared REST quota" on adjacent lines. Nothing in the chain but the name was ever called anything other than fix: the action is `fix`, the report says `fix`, the session is a fix session. The feature is now autofix, which pairs with autoreview: one keeps open pull requests reviewed, the other keeps them fixed. crq drain install -> crq autofix install crq drain [on|off|default] crq autofix [on|off|default] A hard break, with no compatibility path. The persisted keys move with it — drain -> autofix, drains -> autofix_by_host, repo_drain -> repo_autofix — so a pre-rename binary sharing the state ref sees no autofix switches and no dispatch health. Every host has to be upgraded together. The installed artifacts move too: crq-autofix, crq-autofix.service, no.kristofferr.crq-autofix, autofix.log/autofix.err. The invariant is fix-first now, and findings are cleared rather than drained. "Drain" survives only where it means draining a resource or a stream: the REST quota, a child's stdout, git's credential-helper lines. Three fixes the preflight surfaced in the files this touches: - the install plan took its repository list straight off a map, so a re-install rewrote the unit in a new order and --dry-run described a different service than the install that followed it; - Environment= assignments were not percent-escaped, and systemd expands specifiers there, so a '%' in a path or a review command was eaten; - the repository key trimmed ".git" before lowercasing, so a ".GIT" suffix survived into the key and matched nothing.
|
@codex review |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3300dfb3e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The rename is a hard break in the state and the CLI, but a break in naming stops nothing that is already running. A host that ran `crq drain install` keeps an enabled crq-drain unit, and `crq autofix install` only enabled and restarted crq-autofix beside it: two watchers then scan the same fleet, each starting sessions and racing the other's dispatch claims. The install now stops the old one first — `systemctl --user disable --now`, or launchd's disable plus bootout, since bootout alone comes back at the next login. It is detected from the unit still being on disk rather than attempted blindly, so a first install runs no pointless command and needs no failure text to know it was benign, and --dry-run names what it will retire.
1 participant
drainwas carrying three unrelated jobs at once — the installer, the per-repository permission to write code, and the fix-before-review invariant — while the same word, correctly, also meant "consume the shared REST quota" on adjacent lines of the same docs. Nothing else in the chain was ever called anything but fix: the action isfix, the report saysfix, the session is a fix session.The feature is now autofix, which pairs with
autoreview: one keeps open pull requests reviewed, the other keeps them fixed.This is a hard break
No compatibility path, by design. The persisted keys move with the name:
drainautofixdrainsautofix_by_hostrepo_drainrepo_autofixA pre-rename binary sharing the state ref therefore sees no autofix switches and no dispatch health, and vice versa — every host has to be upgraded together. Unknown-member round-tripping means neither side erases the other's keys, so this degrades to "does not see", not "destroys".
The installed artifacts move too:
crq-autofix,crq-autofix.service,no.kristofferr.crq-autofix,autofix.log/autofix.err. An existingcrq-draininstall must be disabled and removed beforecrq autofix install, or two watchers run side by side.Everything else
The invariant is fix-first now, and findings are cleared rather than drained.
drainsurvives only where it means draining a resource or a stream — the REST quota, a child's stdout, git's credential-helper lines.Also fixes the
crq helplayout bug where "install and start unattended autofix" was printed under theon|off|defaultline instead of theinstallline it describes.Three fixes the preflight surfaced
All predate this diff, all in the files it renames:
--dry-rundescribed a different service than the install that followed it. Reproduced on this host before the fix. Now uses the existingsortedRepoList, with a test.Environment=assignments were not percent-escaped. systemd expands specifiers inEnvironment=values, so a%in a path or a review command was eaten or failed to load the unit.$is deliberately left alone — unlikeExecStart, it is literal there..gitbefore lowercasing, so a.GITsuffix survived into the key and matched nothing.The eight remaining preflight findings are skipped: two ask to move dispatch CAS writes out of
watch.gointo the service apply layer (an architectural change, not a rename), and the rest are pre-existing test-strengthening and doc-wording notes unrelated to this diff.Verification
gofmt,go build,go vet,go test ./...clean. Swapped live oncachyos: oldcrq-drain.servicedisabled and removed,crq-autofix.serviceactive and enabled,crq autofixlisting correct.crq drainnow exitsunknown command: drain.