Skip to content

Add test rules learned from the Touch ID work to AGENTS.md - #14

Open
scottjones wants to merge 3 commits into
mainfrom
docs/agents-test-rules
Open

scottjones wants to merge 3 commits into
mainfrom
docs/agents-test-rules

Conversation

@scottjones

Copy link
Copy Markdown
Collaborator

Summary

Adds a Tests section and two rules to AGENTS.md, from mistakes the reviews of the Touch ID branches (#12, omacom/omarchy-mac#694, omacom/omarchy-pkgs#806) kept catching: tests that passed only because the Mac running them had Omarchy installed, a detection path that could hide the old one, an integration test skipping silently in CI, a green CI run that never reached the code, a udev add rule missing the upgrade that installs it, and a once-only invitation that could fire before its dependencies shipped.

 ## Rules
+- udev rule acting on `add` → also a pacman hook, skipped in a chroot or unbooted root
+- a once-only step ships only after everything it leads to is published
+## Tests
+- same result on every machine: never read the host's installs, platform root, /sys or services
+- a new detection path or fallback → test the inverse
+- need a newer runtime → move test/integration/runtime, don't skip
+- check CI actually ran it before calling it tested

Evidence

Documentation only; check-scope and check-architecture-gates pass locally.

Merge Danger

Door: two-way

Blast Radius: docs

🤖 Generated with Claude Code

Reviews of the Touch ID branches kept finding the same kinds of slip: a
test that passed only because the Mac running it had Omarchy installed, a
detection path that could hide the one before it, an integration test that
skipped silently in CI, a green run that never reached the code, a udev
add rule that missed the upgrade installing it, and a once-only invitation
that could fire before its dependencies shipped. AGENTS.md now says so.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@scottjones
scottjones requested a review from maralcbr as a code owner October 5, 2026 14:22
@maralcbr

maralcbr commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

@scottjones good idea to write these down; lines 26 and 28 are precise and fit the file. A few wording fixes so agents don't over-apply them:

  • Line 20 (udev add → pacman hook) is too broad. It would also cover 70-omarchy-mac-external-displays.rules, whose add must only run while the boot splash is up, and change actions that must not be replayed. Suggest: "A udev add action that must reach a device already present also gets a pacman hook that does that work on install and upgrade; the hook exits 0 in a chroot or an unbooted root, and never fails the transaction. An add that should only run when the device appears does not."
  • Line 25 ("never read … /sys") reads as a ban on all /sys reads, while existing tests already point copies at /sys fixtures. Suggest "never the host's /usr/share/omarchy-platform, installed omarchy-* commands, /sys or running services: stage into a temporary root and point copies at fixtures."
  • Line 21 ("a step the owner sees only once") also covers first boot, which already belongs to the installer. "An invitation the owner sees only once ships only after everything it leads to is published" says what was meant.
  • Line 27 ("don't skip") sits next to the existing skip when OMARCHY_TEST_RUNTIME is unset. Say it's about the pin: "moves test/integration/runtime to that commit; it does not skip because that pin lacks it."

Wording from Marcelo's review, so agents don't over-apply them: the hook
rule covers only an add that must reach a device already present, and the
hook never fails the transaction; the once-only rule is about invitations,
not first boot; tests may read /sys fixtures, never the host's; and the
pin rule doesn't touch the skip when OMARCHY_TEST_RUNTIME is unset.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@scottjones

Copy link
Copy Markdown
Collaborator Author

@maralcbr thanks, applied all four in 0fc00085.

@maralcbr

maralcbr commented Oct 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks @scottjones, the rules capture real lessons from the Touch ID work. One change before merge:

  • Line 25: please reword to "stub external dependencies, keeping the real commands under test". As written, "stub the runtime's commands" contradicts how the integration tests work.

Smaller things:

  • "Invitation" isn't defined anywhere else in the repo, so an agent can't act on that rule. A short definition (or a pointer to the fingerprint-setup offer it refers to) would fix it.
  • The PR description no longer matches the diff; worth a refresh.

With the line 25 fix in, this is good to merge from my side.

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