dotfiles/skills/add-application-or-service/references/review-checklist.md

# Application and Service Review Checklist

Use this reference during design and again during final review.

## Ownership and fragment matrix

| Concern                                                  | Owner                               | Typical fragment                          |
| -------------------------------------------------------- | ----------------------------------- | ----------------------------------------- |
| User-facing GUI, CLI, editor, or compositor              | `modules/applications/<name>/`      | `home*.nix`, `nixos.nix`, or `darwin.nix` |
| User-scoped package and program settings                 | Application unit                    | `home.nix` or `home/<class>.nix`          |
| macOS Homebrew package or cask                           | Owning application or service       | `darwin.nix`                              |
| Daemon or long-running service                           | `modules/services/<name>/`          | `nixos.nix` or `darwin.nix`               |
| Firewall, group, permission, or service account          | Owning service                      | `nixos.nix` or `darwin.nix`               |
| Upstream module defining options                         | Owning unit metadata                | `meta.nix` under `imports.<class>`        |
| Technical prerequisite unit                              | Owning unit metadata                | Fully qualified `meta.includes`           |
| A set of independent tools chosen for one purpose        | Narrowest coherent profile          | Profile `meta.includes`                   |
| Machine fact such as UUID, monitor ID, or static address | `hosts/<name>/`                     | Normal host module                        |
| Missing or replaced package                              | `overlays/` only after proving need | Overlay definition                        |

Use `home/common.nix` when Home Manager configuration is truly shared between
NixOS and Darwin. Use `home/nixos.nix` or `home/darwin.nix` for class-specific
Home Manager behavior. Root `common.nix` is system-side and never Home Manager.
Do not create an unused counterpart for symmetry.

## Dependency review

For every edge from unit A to unit B, answer these questions:

1. Does A fail to work without B? If yes, put the fully qualified ID of B in
   A's `meta.includes`.
2. Are A and B merely useful together for a particular workflow? If yes, let a
   profile include both.
3. Does the dependency exist only on one host? Keep the machine fact in the
   host, but keep reusable behavior in its unit or profile.
4. Is a new `specialArgs`, `_module.args`, Registry field, or direct
   `my.*.enable` assignment being proposed? Reject it unless the repository's
   normal module and `meta.includes` mechanisms provably cannot model the
   requirement.
5. Would adding the dependency make the depended-on application select a
   compositor, desktop, personal workload, or unrelated tool? Reverse or remove
   the edge; application metadata contains technical requirements, not taste.

Accept no circular dependency, shortened unit ID, duplicate include, stale
unit ID, or ordering-dependent override.

## Minimality and code-quality review

Reject any of these patterns:

- Empty or placeholder reserved fragments.
- Hand-written enable options or guards already generated by the Registry.
- Top-level `imports`, `options`, or `config` in a configuration fragment.
- External module imports hidden in a guarded configuration fragment.
- Helper files assumed to be auto-imported.
- A helper abstraction used once without reducing meaningful complexity.
- Configuration duplicated across fragments when a shared fragment can express
  it cleanly.
- Direct host application or unit selection where a profile expresses the
  concern.
- A new flake input or overlay when the locked package set already provides the
  package and required module.
- Global argument injection for a value owned by one unit.
- `lib.mkForce` used to win an ordering fight instead of resolving ownership.
- Secret material, generated state, machine IDs, or mutable user preferences
  committed as reusable configuration.
- Comments that repeat the code, compatibility aliases, dead options, or
  opportunistic unrelated cleanup.

Prefer standard upstream module options over hand-written service definitions.
Prefer existing repository arguments and helpers over new plumbing. Preserve
user-owned mutable state unless the requested policy explicitly owns it.

## Validation matrix

Use the `validate-nix-change` skill and run checks from the repository root.
Keep exact results for the handoff. The validation app never switches or
activates a live system.

### Always

1. Run `nix run .#check -- plan --paths <task-path>... --json` and inspect the
   affected units and hosts.
2. Run `nix run .#check -- fast --paths <task-path>...` during implementation.
3. Review `git status --short`, `git diff --stat`, and the complete task diff.
4. Run `nix run .#check -- all --paths <task-path>...` before handoff. It runs
   all-system evaluation and compatible targeted builds without activation.
5. Reserve `nix run .#check -- full` for CI, scheduled maintenance, or an
   explicit repository-wide audit.

Do not run `nh os switch`, `nixos-rebuild switch`, `darwin-rebuild switch`,
`home-manager switch`, or an equivalent activation command.

### NixOS or Home Manager on NixOS

- Confirm that the validation plan includes each affected real NixOS host.
- Build affected NixOS configurations with `--no-link` through the validation
  app when the current system supports them.
- Inspect the resulting option that proves selection: for example
  `environment.systemPackages`, the user's `home.packages`,
  `systemd.services`, `users.users.<name>.extraGroups`, or the upstream
  `programs`/`services` option.

### nix-darwin or Home Manager on Darwin

- Confirm that every affected Darwin host is evaluated even when running on
  Linux.
- Inspect `homebrew.casks` or `homebrew.brews` for Homebrew-backed additions.
- Evaluate the relevant Home Manager program or package option.
- Build a Darwin configuration only on a compatible Darwin runner or builder;
  otherwise report the build as an explicit platform gap.

### Profiles and cross-platform changes

- Confirm transitive selection through `meta.includes`, not only direct
  mentions. The validation plan computes reverse dependency closure.
- Evaluate every real host selecting the changed profile.
- Evaluate both host classes for a cross-platform profile, even if only one
  current fragment changed.
- Re-read `modules/profiles/README.md` and `hosts/default.nix` for stale meaning,
  compatibility, or role statements.
- If no real host selects the new unit, construct a non-persistent evaluation
  that enables it or explain why the unit is intentionally dormant. Do not add
  a fake host or permanent direct selection as a test harness.

### Runtime-dependent behavior

Evaluation and builds cannot prove GUI appearance, credentials, network access,
hardware behavior, successful daemon interaction, or reboot state. For
reusable NixOS behavior, use the `test-nixos-service` skill and add a
`pkgs.testers.runNixOSTest` check. State the precise manual post-activation check
needed for physical hardware or external systems. Never describe evaluation as
a runtime test.