feat(agentic-engineering)!: merge spend stewardship into the engineer - #90
Conversation
Spend stewardship becomes a dimension of the automated-ai-engineer entrypoint instead of a separate FinOps role with its own schedule. Running cost is incurred by the code the engineer already owns, so a second writer over the same repositories produced overlapping claim lanes and a duplicate copy of the delivery/review/merge discipline — while the consumer contract already had to declare that both roles share one provider instance. The generic money boundaries now live in the reviewed definition: value per unit cost over cost reduction, the protected-outcomes floor as a veto, never move money, no personalised investment advice, no private financial data in a public artifact, read-only against production. The deployment's own money facts stay consumer-owned behind a new conditionally-required "Spend contract" section; absent it, the engineer fails closed on the cost dimension only. validate-manifests.sh now pins those boundaries to the entrypoint and rejects a resurrected finops-engineer role or schedule, so the old two-writer shape fails closed rather than being redeployed alongside the merged one. Self-test covers each new guard: 108 passed, 0 failed. BREAKING CHANGE: removes spec.roles["finops-engineer"] and its schedule, and renames spec.consumer.requiredWhenFinOpsEnabled to requiredWhenSpendStewardshipEnabled (["Spend contract"]). Consumers must retire the finops-engineer schedule and update their desired-state copy.
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughVersion 3 consolidates spend stewardship into the Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
![coderabbitai[bot]](https://cdn.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/agentic-engineering/README.md`:
- Around line 23-32: Update the v3 installation sequence in the README so the
existing finops-engineer schedule is quiesced or atomically replaced before
enabling or reconciling the v3 engineer schedule. Use the runtime’s native
scheduler control to retire finops-engineer first, preventing any overlap
between the old schedule and the new engineer loop.
In `@README.md`:
- Around line 93-95: Update README.md lines 93-95 to direct existing
automated-ai-engineer installations through the v2 migration and then the v3
migration, or a combined migration path, before their next scheduled run. Update
plugins/agentic-engineering/README.md lines 56-57 to require installed version
3.0.0 or later before unattended writes are re-enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bffec300-1499-464b-98d1-83d030de9de9
📒 Files selected for processing (12)
.claude-plugin/marketplace.json.github/plugin/marketplace.jsonREADME.mddocs/adr/0004-consolidate-agentic-engineering.mddocs/adr/0005-merge-spend-stewardship-into-the-engineer.mdplugins/agentic-engineering/.claude-plugin/plugin.jsonplugins/agentic-engineering/README.mdplugins/agentic-engineering/agents/automated-ai-engineer.agent.mdplugins/agentic-engineering/plugin.jsonplugins/agentic-engineering/resources/provider-neutral.desired-state.jsonscripts/validate-manifests.shscripts/validate-manifests.test.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
plugins/**/plugin.json
📄 CodeRabbit inference engine (AGENTS.md)
Each
plugin.jsonmust use a kebab-casenamematching^[a-z0-9-]+$, includedescriptionandversion, and omit component-path fields such asskillsandagents; resources are auto-discovered.
Files:
plugins/agentic-engineering/plugin.json
README.md
📄 CodeRabbit inference engine (AGENTS.md)
Keep the README plugin table in lockstep with the manifests and filesystem: every plugin must have one row, every row must represent a plugin, and the Resources column must match bundled skills, MCP server keys, and custom agents.
Files:
README.md
plugins/**/agents/*.agent.md
📄 CodeRabbit inference engine (AGENTS.md)
Custom agents must use the
.agent.mdsuffix and YAML frontmatter containing non-emptynameanddescriptionfields.
Files:
plugins/agentic-engineering/agents/automated-ai-engineer.agent.md
plugins/**/resources/*.desired-state.json
📄 CodeRabbit inference engine (AGENTS.md)
Desired-state resource documents must use the provider-neutral schema, contain the required consumer contract and no placeholders, and be explicitly linked from the owning plugin README; they do not count as auto-discovered resources.
Files:
plugins/agentic-engineering/resources/provider-neutral.desired-state.json
scripts/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
Keep
scripts/validate-manifests.shas the single source of truth for manifest, plugin, README, desired-state, and skill-provenance validation; do not weaken checks to pass.
Files:
scripts/validate-manifests.test.shscripts/validate-manifests.sh
🪛 LanguageTool
docs/adr/0005-merge-spend-stewardship-into-the-engineer.md
[style] ~55-~55: Consider an alternative for the overused word “exactly”.
Context: ...g a mandate into a larger definition is exactly where a boundary gets quietly droppe...
(EXACTLY_PRECISELY)
[style] ~98-~98: Consider using “who” when you are referring to a person instead of an object.
Context: ...cted because it would stop a consumer that does not want cost work from using the ...
(THAT_WHO)
plugins/agentic-engineering/README.md
[style] ~34-~34: Consider using “who” when you are referring to a person instead of an object.
Context: ...o writers over one concern. A consumer that keeps its FinOps definition as a separa...
(THAT_WHO)
🔇 Additional comments (12)
docs/adr/0005-merge-spend-stewardship-into-the-engineer.md (1)
1-102: LGTM!docs/adr/0004-consolidate-agentic-engineering.md (1)
7-10: LGTM!plugins/agentic-engineering/README.md (1)
14-21: LGTM!Also applies to: 34-37, 70-71, 96-112, 139-143, 153-160
README.md (1)
17-17: LGTM!Also applies to: 89-92, 172-177
.claude-plugin/marketplace.json (1)
29-30: LGTM!.github/plugin/marketplace.json (1)
29-30: LGTM!plugins/agentic-engineering/.claude-plugin/plugin.json (1)
3-4: LGTM!plugins/agentic-engineering/plugin.json (1)
3-4: LGTM!plugins/agentic-engineering/agents/automated-ai-engineer.agent.md (1)
5-16: LGTM!Also applies to: 49-61, 70-72, 117-190
plugins/agentic-engineering/resources/provider-neutral.desired-state.json (1)
33-34: LGTM!Also applies to: 61-61, 94-94, 118-118
scripts/validate-manifests.sh (1)
437-437: LGTM!Also applies to: 570-595, 654-670, 722-723, 738-761, 788-795
scripts/validate-manifests.test.sh (1)
566-571: LGTM!Also applies to: 625-626, 640-642, 656-659, 694-698, 899-946
…ng v3 Both fixes address CodeRabbit findings on #90. The migration had the ordering backwards. Installing v3 while the old finops-engineer schedule is still armed opens exactly the concurrent-stewardship window the merge closes — two writers proposing against the same spend. Retiring the schedule first risks at most one missed cost pass, which is cadence-gated and far cheaper. Quiesce or atomically replace it ahead of the v3 engineer schedule. The root onboarding route also sent existing installations only through the v2 checklist, whose pre-write verification accepted 2.0.0 — permitting writes to resume with the retired schedule live. It now requires both migrations, and the verification requires 3.0.0 or later.
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
Readiness record — promotingProgrammatically tested.
Negative controls — every new guard proven to fire. RED evidence, not assumed: removing each of Two of my first probes reported "not caught" and both were broken probes, not missing guards — a Tried and evaluated as a user. This change is a reviewed agent definition plus manifest/validator code, so I exercised the surfaces it actually has: ran the validator and its self-test as CI does; confirmed the merged entrypoint still parses as frontmatter + body with non-empty Reviewed. CodeRabbit at Pentad at |
1 participant
Why
We ran two agents over one portfolio. The FinOps engineer owned the money and the primary engineer
owned the code — but the money is spent by that code, so both were selecting work from the same
repositories, claiming the same issues, and carrying their own copy of the same review-and-merge
discipline. The consumer contract had already conceded the point by recording that both roles share
one provider instance. A cost pass on its own schedule also can't see what the engineer is about to
change, and the engineer can't see what it costs.
What
Spend stewardship becomes a dimension of the
automated-ai-engineerentrypoint rather than a separaterole, so one agent measures the spend, decides the change, and ships it in one loop.
The generic money boundaries move into the reviewed definition — value per unit cost rather than cost
reduction, the protected-outcomes floor as a veto, never move money, no personalised investment
advice, no private financial data in a public artifact. The deployment's own money facts (price
sources, its floor, the private channel) stay consumer-owned behind a new conditionally-required
Spend contract section; without it the engineer runs normally and simply does no spend work.
CI now enforces the merged boundary: it pins those limits to the entrypoint and rejects a resurrected
FinOps role or schedule, so the old two-writer shape can't quietly come back alongside the new one.
Breaking for consumers — the
finops-engineerschedule must be retired and the desired-state copyupdated. Version 3.0.0, with the migration steps in the plugin README. Land this before the
devantler-tech/monorepoconsumer PR that retires its local FinOps agent.Decision record: ADR 0005, superseding
ADR 0004 points 5 and 8. Maintainer-directed in session, so there is no originating issue.