[nick-potts]

Why

Goal: untangle the core/ workspace so pieces that change together live together, with behavior unchanged. Later agent changes then read less, break less silently and debug faster. This is batch 1 of a 25-batch plan, kept to one small chunk so it lands fast beside other PRs.

The config_removal_mark SQL in ployz-store was written by hand in seven modules, with the state literals typed each time. This PR gives it one owner.

What changed

• removal::mark now owns every statement over config_removal_mark. It gains unmarked, marks_holding and drop_live. The state literals come from state_text and one REMOVED constant. pull_request, branch::follow and branch::proposal call these and no longer spell the SQL.
• The verify-removal-marks skill lists the new mark.rs helpers and their callers.
• One test in environment_intent pins that a stored Service slug that is not a Service Name ("Web", "web_app", "web-", 64 characters) still decodes.

Scope

• T11-F1 is deferred. It changed SavedServiceIntent.slug from String to ServiceName and removed about 30 re-parse sites. It is taken back out of this PR, because the stricter decode leaves behavior worse than main. Main decodes and admits a stored intent with slug "Web", "web_app", "web-" or a 64-character lowercase slug. With the type change those intents fail to decode (invalid Service Name "Web": a 1-63 character lowercase DNS label). No writer on main is known to produce such a slug, but deployed Cloud rows cannot be inspected from here, and an environment whose stored intent no longer decodes is worse than main. F1 needs a data check on deployed rows, or a lenient-decode design, first.
• The nodes(), node_by_*, volume_of/config_of and NodeKinds additions stay for a follow-up. They touch most of the same files as other open PRs.
• The spec names the merge-undo helper cancel_live. It is drop_live, because it deletes the live mark and leaves no tombstone. That is not a cancel in the mark's own Cancelled sense.
• Open question, left as is: a Project's removal stops idle close but not PR follow. The Environment-only predicate keeps its old meaning. Whether a Project removal should also stop follow is a separate decision.

Design bets: bet 8 only, and it is unchanged: a removing Environment is still never listed as open. The Stable promise is untouched. No row, replicated body or serialized shape changes.

Blast Radius

Cloud-side Config Store code only. No Machine behavior changes, so no Boat run. Every read and write behaves as on main.

Verification

• cargo clippy -p ployz-store -p ployz-core --all-targets -- -D warnings and cargo fmt --check are clean on this head.
• cargo nextest run -p ployz-store filtered to removal, mark, pull_request, branch and sweep passes on SQLite (286 tests). cargo test -p ployz-core --test environment_intent --test sync and cargo test -p ployz-verify removal_marks pass.
• The Postgres 16 run of the same filter (291 tests, recipe in core/AGENTS.md) was done on the earlier head, before F1 was taken out. It has not been rerun on this head. The config_removal_mark statements are unchanged since then.
• New test: a_stored_service_slug_that_is_not_a_service_name_still_decodes.

Decision log: T11-F2, and why T11-F1 is deferred

T11-F2. removal::mark owns every SQL statement over config_removal_mark. It adds unmarked(environment_column) (the NOT EXISTS ... state = 'removing' fragment), marks_holding(environment_column) (the Project-aware scalar IDLE_FACTS uses) and drop_live, which deletes the live mark for the PR merge undo. State literals come from state_text(State::..) in mark.rs only. Behavior is preserved exactly: the Environment-only copies keep the Environment-only predicate, IDLE_FACTS keeps the Project-aware one. Teardown deleting marks by cascade stays as it is. Bets touched: bet 8 only, unchanged. Frozen risk: none.

T11-F1 (deferred). The idea was to have the saved document carry a Service's name as a ServiceName, so the store stops re-parsing it. The serde form is try_from/into String, so JSON stays byte-identical, but the decode becomes strict. Review replaced only /services/0/slug in the environment_intent fixture and found that main decodes and admits "Web", "web_app", a 64-character lowercase slug and "web-", and the typed head fails to decode them. That is worse than main for any such stored row. It returns after a data check on deployed rows or a lenient-decode design.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 12171d54-21d5-486b-97dd-68f5295c60cf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

• 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