fix(daemon): reject a non-map agent spec profile instead of crashing normalize

[rudycelekli]

What a user gets

Before: an agent spec with a bare profile key

profiles:
  default:

validated as valid: true and then normalizeAgentSpec threw TypeError: Cannot read properties of null (reading 'uses'), so the failure surfaced later, at import or launch, instead of in validation. After: validateAgentSpec reports profiles.default: must be a map (an empty profile is {}). default: {} still validates and normalizes.

How you verified it

Tested at upstream main (fetched and rebased) plus this change, Node 22, npm ci --ignore-scripts -w packages/daemon.

New packages/daemon/test/agent-spec-null-profile.test.ts (5 cases: bare key, scalar, empty list, plain string, and the accepted {}):

before the fix (src reverted): Tests 4 failed | 1 passed (5)
after the fix:                 passes; with agent-manifest.test + agent-manifest-plugins.test: Tests 67 passed (67)

Also: all 19 shipped agent.yaml specs still validate (invalid: 0 of 19); the 42 daemon test files matching agent|pod|profile: 612 passed, 2 failed. The 2 failures are in pod-bundle-assembler.test.ts ("keeps in-root symlinks with a symlinked rig root") and fail identically on unmodified main in my environment. tsc --noEmit -p packages/daemon/tsconfig.json clean.

Not run: npm run build/lint for the other packages, the full npm test, a live import/launch.

Anything you were unsure about

• I chose to reject a non-map profile rather than treat default: as an empty profile: a null profile could never normalize, so no working spec can contain one, but this also now rejects a scalar or list profile (default: 5), which previously slipped through as an empty profile. If you would rather accept a bare key as {}, that is a two-line change in normalizeAgentSpec instead; happy to switch.

• The new guard sits in the existing profile loop, wrapped in a bare block to keep the diff small rather than re-indenting the body.

• One concern per PR; no version bump; no CHANGELOG.md edit

• Tests added or updated where the change is testable

• I listed the checks I ran, their results, and any checks I could not run

(Checkboxes left for the author to tick.)

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Bare default: profiles now validate and normalize the same way as empty profiles. Existing profile content and other profile values retain their expected behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: efd8eb42-6ca5-4ae0-99b6-a333a3bb11cc

📥 Commits

Reviewing files that changed from the base of the PR and between 63f7b4c and 70b98e8.

📒 Files selected for processing: 2

• packages/daemon/src/domain/agent-manifest.ts
• packages/daemon/test/agent-spec-null-profile.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 3 remain after this review.

📝 Walkthrough

normalizeAgentSpec now treats null profile values as empty profiles before normalization. New tests cover bare, explicitly empty, scalar, list, and populated profile values.

Changes: Profile Normalization

Layer / File(s)	Summary
Normalize and test profile values packages/daemon/src/domain/agent-manifest.ts, packages/daemon/test/agent-spec-null-profile.test.ts	normalizeAgentSpec substitutes an empty object when a profile value is null. Tests cover normalization of bare, empty, scalar, list, and populated profiles.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: mvschwarz

Merge Risk: ⚪ Minimal · up to 70b98

Bare default: profiles now normalize like {} instead of throwing, and the reviewed change presents no actionable merge risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Title check		The title is related to agent profile normalization, but it states that the change rejects non-map profiles. The implementation instead treats a null profile as an empty profile and preserves existing…	Update the title to describe normalizing null agent profiles as empty profiles, for example: "fix(daemon): normalize null agent profiles as empty profiles".

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title is related to agent profile normalization, but it states that the change rejects non-map profiles. The implementation instead treats a null profile as an empty profile and preserves existing scalar and list handling.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mvschwarz]

Thanks, @rudycelekli. You found a bare profile key (profiles: default: with nothing after it) that validates and then crashes normalizeAgentSpec with a TypeError, wrote the five-case test, and said plainly which inputs your choice would start refusing.

One change, please: switch to the alternative you offered. Drop the new check in validateAgentSpec, and instead normalize a bare profile key as an empty profile, the same as default: {}, so the crash goes away without a new validation error. Please keep main's current handling of a scalar or list profile (default: 5, default: []). As your description says, those already go through as an empty profile. From reading the code, main's normalizeAgentSpec passes each profile value straight to normalizeProfile in agent-manifest.ts, so a fix that handles only the missing (null) value leaves them as they are today. Apart from the bare key, we're not asking for a new refusal or a new coercion rule.

A few small test cases are enough: a bare key, {}, a scalar, a list and a normal named profile, with the existing agent-manifest tests still passing.

When you've pushed it and marked the PR ready, we'll review the change.

[rudycelekli]

Switched to the alternative in 70b98e8. The new validateAgentSpec check is gone (that function is back to main's). normalizeAgentSpec now passes profileRaw ?? {} to normalizeProfile, so a bare default: normalizes as an empty profile, identical to default: {}. Scalar and list profiles (default: 5, default: []) are untouched and keep main's handling (they validate and normalize to an empty profile).

agent-spec-null-profile.test.ts now covers a bare key, {}, a scalar, a list and a normal named profile. The bare-key case failed before the change (validate ok, normalize threw) and passes now; the new test file plus agent-manifest.test.ts and agent-manifest-plugins.test.ts: 67 passed. This PR stays a draft for now.

[openrig-review]

Approved at 70b98e8. A bare profile key (profiles: default: with nothing after it) now normalizes as an empty profile, the same as default: {}, instead of crashing when the agent is resolved or bundled. Validation is unchanged, and scalar and list profiles keep main's handling. Required CI passed at this head, and no main commit has touched its files since its base.

— dev60-planner@v-openrig-build

[mvschwarz]

Thanks, @rudycelekli; #1320 is merged. A bare profile key in an agent spec (profiles: default: with nothing after it) now normalizes as an empty profile, the same as default: {}, instead of crashing later wherever the agent is resolved. Validation and the handling of scalar and list profiles stay as they were. Thanks for switching to the alternative you'd offered. It isn't in a published release yet.