refactor(injectappmodel): EnsureServiceEntry + unify claim semantics by buke · Pull Request #251 · choysum-dev/choysum

buke · 2026-08-06T07:53:51Z

User description

Summary

PR-P1 from the local terminology×injectappmodel redesign: remove Spec ForeignClaimOnOwnerReinject; owner reinject never steals a foreign process claim (NeedInject without ScheduledApp).
Add Spec EnsureServiceEntry + Plan NeedEnsureServiceEntry Decide stub (no virtual service Effects yet — that is PR-P2).
Builtin FieldDefault/AppSetting keep EnsureServiceEntry=false (empty ServiceEntryPoint still skips).

Test plan

go test ./internal/module/artifact/build/injectappmodel/ -count=1
go test ./internal/module/artifact/build/... ./internal/module/lifecycle/ -count=1
Confirm no .gitignore / .dev/docs changes in this PR

Made with Cursor

PR Type

Enhancement, Tests

Description

Unify owner-reinject claim semantics across all Specs in Go core.
Introduce EnsureServiceEntry Spec field and NeedEnsureServiceEntry Plan flag stub.
Maintain empty service entry skip behavior for built-in FieldDefault and AppSetting models.
Expand Go unit tests for unified claims and service entry flag handling.

File Walkthrough

Relevant files

Tests

coverage_test.go `Update coverage test for unified registry claim logic` internal/module/artifact/build/injectappmodel/coverage_test.go Updated test setup to remove reliance on the deprecated `ForeignClaimOnOwnerReinject` field.	+1/-1
injectappmodel_test.go `Add test coverage for EnsureServiceEntry and claim behavior` internal/module/artifact/build/injectappmodel/injectappmodel_test.go Consolidated foreign claim owner-reinject test cases for FieldDefault and AppSetting. Added tests verifying empty service entry skipping and `EnsureServiceEntry` flag behavior.	+66/-28

Enhancement

inject.go `Implement EnsureServiceEntry plan handling and unified claim rules` internal/module/artifact/build/injectappmodel/inject.go Evaluates `EnsureServiceEntry` on `Spec` during plan decision. Sets `NeedEnsureServiceEntry` on `Plan` when service entry point is missing. Standardizes `claimNeedInject` to prevent claim stealing across all Specs. Skips file materialization during P1 stub phase when `NeedEnsureServiceEntry` is set.	+24/-10
plan.go `Add NeedEnsureServiceEntry field to Plan struct` internal/module/artifact/build/injectappmodel/plan.go Added `NeedEnsureServiceEntry` boolean field to the `Plan` struct.	+4/-0
spec.go `Replace ForeignClaimOnOwnerReinject with EnsureServiceEntry in Spec` internal/module/artifact/build/injectappmodel/spec.go Replaced `ForeignClaimOnOwnerReinject` field with `EnsureServiceEntry` on `Spec`.	+4/-3

Refactoring

registry.go `Remove ForeignClaimOnOwnerReinject from AppSetting default spec` internal/module/artifact/build/injectappmodel/registry.go Removed `ForeignClaimOnOwnerReinject` from default `AppSetting` model spec.	+7/-6

Summary by CodeRabbit

New Features
- Added support for ensuring required service entries when injection is enabled but no entry currently exists.
- Improved owner reinjection behavior by attempting claims consistently while avoiding duplicate application scheduling.
Bug Fixes
- Prevented unnecessary injection when service entries are absent and assurance is not enabled.
- Improved handling when another module already owns the relevant claim.
Documentation
- Clarified service-entry behavior and configuration expectations.

- Replace ForeignClaimOnOwnerReinject with fixed owner-reinject claim rules for all Specs. - Add EnsureServiceEntry and NeedEnsureServiceEntry Decide stub (virtual entry materialize deferred to P2). - Keep FieldDefault/AppSetting skipping inject when ServiceEntryPoint is empty. Co-authored-by: Cursor <cursoragent@cursor.com>

coderabbitai · 2026-08-06T07:54:25Z

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 38 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f812a515-fe1f-4155-8ea1-ed8c5eae94e7

📥 Commits

Reviewing files that changed from the base of the PR and between 8c80ca3 and 9768c9e.

📒 Files selected for processing (3)

internal/module/artifact/build/injectappmodel/inject.go
internal/module/artifact/build/injectappmodel/injectappmodel_test.go
internal/module/artifact/build/injectappmodel/spec.go

📝 Walkthrough

Walkthrough

The injection model replaces ForeignClaimOnOwnerReinject with EnsureServiceEntry. Planning records when service-entry creation is required, defers materialization without an entry point, and always attempts owner reinjection claims.

Changes

Service-entry planning

Layer / File(s)	Summary
Service-entry contracts and defaults `internal/module/artifact/build/injectappmodel/spec.go`, `internal/module/artifact/build/injectappmodel/plan.go`, `internal/module/artifact/build/injectappmodel/registry.go`	`Spec` uses `EnsureServiceEntry`. `Plan` exposes `NeedEnsureServiceEntry`. Default specifications document empty service-entry behavior.
Planning, claims, and materialization `internal/module/artifact/build/injectappmodel/inject.go`	Planning can continue without a service entry when ensuring is enabled. Foreign owner claims no longer schedule an application. Materialization defers effects while the entry is absent.
Planning behavior validation `internal/module/artifact/build/injectappmodel/injectappmodel_test.go`, `internal/module/artifact/build/injectappmodel/coverage_test.go`	Tests cover owner reinjection claims, empty service-entry handling, and deferred ensuring behavior.

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

Possibly related PRs

choysum-dev/choysum#239: Related AppSetting/FieldDefault injection planning and owner-reclaim behavior.
choysum-dev/choysum#249: Directly related injectappmodel planning, registry specifications, and tests.

Suggested labels: Review effort 2/5

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly identifies the injectappmodel refactor, service-entry ensuring, and unified claim semantics.
Description check	✅ Passed	The description is detailed, on-topic, and includes the change summary, test plan, scope, and file-level walkthrough.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch refactor/injectappmodel-ensure-service-entry-p1

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

github-actions · 2026-08-06T07:54:47Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

github-actions · 2026-08-06T07:55:08Z

PR Code Suggestions ✨

No code suggestions found for the PR.

codecov · 2026-08-06T07:57:52Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

…flag - Derive ensure-needed from Spec.EnsureServiceEntry plus empty ServiceEntryPoint at Materialize time. - Keep Decide allowing NeedInject when EnsureServiceEntry is set without a separate Plan bit. Co-authored-by: Cursor <cursoragent@cursor.com>

buke · 2026-08-06T07:59:58Z

@cursor review

cursor

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 9768c9e. Configure here.}

github-actions Bot added the Review effort 2/5 label Aug 6, 2026

cursor Bot reviewed Aug 6, 2026

View reviewed changes

buke merged commit a711242 into main Aug 6, 2026
44 checks passed

buke deleted the refactor/injectappmodel-ensure-service-entry-p1 branch August 6, 2026 08:04

coderabbitai Bot mentioned this pull request Aug 6, 2026

feat(injectappmodel): inject TranslationTerm MetaModel per app #252

Merged

4 tasks

Navigation Menu

Uh oh!

refactor(injectappmodel): EnsureServiceEntry + unify claim semantics - #251

refactor(injectappmodel): EnsureServiceEntry + unify claim semantics#251
buke merged 2 commits into
mainfrom
refactor/injectappmodel-ensure-service-entry-p1

buke commented Aug 6, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

github-actions Bot commented Aug 6, 2026

Uh oh!

codecov Bot commented Aug 6, 2026 •

edited

Loading

Uh oh!

buke commented Aug 6, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

buke commented Aug 6, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Summary

Test plan

PR Type

Description

File Walkthrough

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

github-actions Bot commented Aug 6, 2026

PR Reviewer Guide 🔍

Uh oh!

github-actions Bot commented Aug 6, 2026

PR Code Suggestions ✨

Uh oh!

codecov Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

buke commented Aug 6, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

buke commented Aug 6, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading

codecov Bot commented Aug 6, 2026 •

edited

Loading