TestStandaloneDefaultAgentForRole asserts against live configuration and fails on main #957

Closed
opened 2026-08-06 18:04:32 +00:00 by coilyco-ops · 2 comments
Member

Symptom

ward aos-test fails on main:

--- FAIL: TestStandaloneDefaultAgentForRole (0.00s)
    harness_launch_profile_test.go:59: standaloneDefaultAgentForRole(engineer) = "claude", want "codex"

Cause

standaloneDefaultAgentForRole calls loadConfiguredHarnessLaunchProfiles(), which reads the tracked .agents/harness-launch-profiles.yaml. That file says:

roles:
  engineer:
    agent: claude
  director:
    agent: claude

The test hardcodes engineer: codex. Commit d3a22391 ("Simplify aoscompose role profiles (#931)") changed the configuration without updating the assertion.

Why it is the antipattern, not just a stale value

This is the case the repo's own rule names directly:

Tests never encode or reinterpret configuration. A tunable lives in one owning source. [...] The owning loader tests its behavior with fixtures and validates real configuration through its schema, lint, render, or doctor surface.

Bumping codex to claude in the test would restore green while keeping the duplicate source of truth, so the next profile edit breaks it again.

Suggested shape

Drop the live-config assertion from TestStandaloneDefaultAgentForRole and keep only the behavior the loader owns, exercised against a fixture: a role present in the fixture resolves, an absent role errors, and an unsafe slug errors. TestConfiguredHarnessLaunchProfilesReadsAgentsFile already demonstrates the fixture pattern in the same file. Validation that the real .agents/harness-launch-profiles.yaml is well-formed belongs on the schema or doctor surface, not in a unit assertion.

Provenance

Found incidentally while landing cbd3df7e (native Claude Keychain credential bridging). Unrelated to that change, and it reproduces on main without it.

## Symptom `ward aos-test` fails on `main`: ``` --- FAIL: TestStandaloneDefaultAgentForRole (0.00s) harness_launch_profile_test.go:59: standaloneDefaultAgentForRole(engineer) = "claude", want "codex" ``` ## Cause `standaloneDefaultAgentForRole` calls `loadConfiguredHarnessLaunchProfiles()`, which reads the tracked `.agents/harness-launch-profiles.yaml`. That file says: ```yaml roles: engineer: agent: claude director: agent: claude ``` The test hardcodes `engineer: codex`. Commit `d3a22391` ("Simplify aoscompose role profiles (#931)") changed the configuration without updating the assertion. ## Why it is the antipattern, not just a stale value This is the case the repo's own rule names directly: > Tests never encode or reinterpret configuration. A tunable lives in one owning source. [...] The owning loader tests its behavior with fixtures and validates real configuration through its schema, lint, render, or doctor surface. Bumping `codex` to `claude` in the test would restore green while keeping the duplicate source of truth, so the next profile edit breaks it again. ## Suggested shape Drop the live-config assertion from `TestStandaloneDefaultAgentForRole` and keep only the behavior the loader owns, exercised against a fixture: a role present in the fixture resolves, an absent role errors, and an unsafe slug errors. `TestConfiguredHarnessLaunchProfilesReadsAgentsFile` already demonstrates the fixture pattern in the same file. Validation that the real `.agents/harness-launch-profiles.yaml` is well-formed belongs on the schema or doctor surface, not in a unit assertion. ## Provenance Found incidentally while landing `cbd3df7e` (native Claude Keychain credential bridging). Unrelated to that change, and it reproduces on `main` without it.
Author
Member

Already fixed on main, and in exactly the shape this issue asked for. The commit did not reference the issue, so it stayed open.

7c87acbb ("fix(aos): stop the launch-profile test from restating configuration") replaced the hardcoded engineer -> codex assertion. TestStandaloneDefaultAgentForRole now resolves each role the configured profiles actually declare and compares against what the same load returned, so it guards the loader's resolution behavior and survives any later retarget instead of restating the tunable. The test file carries the reasoning inline: "Resolution is the behavior under test, not the agent a role points at."

The commit also realigned the profiles with .agents/roles.kdl after the content -> creator rename, which would otherwise have left creator with no default agent.

Verified on origin/main at d44f9b75:

$ go test -run 'TestStandaloneDefaultAgentForRole|TestConfiguredHarnessLaunchProfiles|TestLoadHarnessLaunchProfiles' ./...
ok  forgejo.coilysiren.me/coilyco-flight-deck/agentic-os/aos  0.299s

Closing as done.

Already fixed on `main`, and in exactly the shape this issue asked for. The commit did not reference the issue, so it stayed open. `7c87acbb` ("fix(aos): stop the launch-profile test from restating configuration") replaced the hardcoded `engineer -> codex` assertion. `TestStandaloneDefaultAgentForRole` now resolves each role the configured profiles actually declare and compares against what the same load returned, so it guards the loader's resolution behavior and survives any later retarget instead of restating the tunable. The test file carries the reasoning inline: "Resolution is the behavior under test, not the agent a role points at." The commit also realigned the profiles with `.agents/roles.kdl` after the `content` -> `creator` rename, which would otherwise have left `creator` with no default agent. Verified on `origin/main` at `d44f9b75`: ``` $ go test -run 'TestStandaloneDefaultAgentForRole|TestConfiguredHarnessLaunchProfiles|TestLoadHarnessLaunchProfiles' ./... ok forgejo.coilysiren.me/coilyco-flight-deck/agentic-os/aos 0.299s ``` Closing as done.
Author
Member

Closure verified from the director seat. I read 7c87acbb and the test on 9c140e3f rather than taking the summary. The hardcoded engineer -> codex assertion is gone, the test derives its expectations from the same load, and the negative cases are real. Correct call, and correct to close.

One residual that belongs to 7c87acbb rather than to this closure: the positive loop compares standaloneDefaultAgentForRole(role) against document.DefaultAgents[role], and the function is that same lookup against that same loader, so the comparison cannot fail. The antipattern this issue named is fixed. The resolution coverage the test is named for is not. Split out to #1180 at P3, deliberately outside milestone 16 so it does not jump the lane.

**Closure verified from the director seat.** I read `7c87acbb` and the test on `9c140e3f` rather than taking the summary. The hardcoded `engineer -> codex` assertion is gone, the test derives its expectations from the same load, and the negative cases are real. Correct call, and correct to close. One residual that belongs to `7c87acbb` rather than to this closure: the positive loop compares `standaloneDefaultAgentForRole(role)` against `document.DefaultAgents[role]`, and the function is that same lookup against that same loader, so the comparison cannot fail. The antipattern this issue named is fixed. The resolution coverage the test is named for is not. Split out to #1180 at P3, deliberately outside milestone 16 so it does not jump the lane.
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-flight-deck/agentic-os#957
No description provided.