Skip to content

fix(retirement): give new journals an independent field owner - #5045

Merged
devantler merged 3 commits into
mainfrom
codex/backstage-journal-ownership-5043
Oct 19, 2026
Merged

devantler merged 3 commits into
mainfrom
codex/backstage-journal-ownership-5043

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

A retirement journal can inherit the application reconciler's ownership and block later deployments when that reconciler tries to remove it. Future operations need their own journal ownership.

What

Give future retirement journals a dedicated owner while preserving their existing identity checks and protections. Existing journal cleanup stays with the separate terminal-maintenance proposal in #5044.

Fixes #5043

⚠️ Merge order: land #5040 first so the retirement admission probes remain non-mutating.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Validated head: 88ed37971586a0aa840b3fcac40f16a58b1da752.

The complete guarded entrypoint checks that the writer names its own field manager. An anonymous loopback API then exercises the real kubectl PATCH, checks the dedicated manager query parameter, and rejects a changed namespace UID/resource-version test or journal binding. Removing the new manager flag reproduced a failure against that native request control; restoring it passed.

Go 1.26.9 affected-package race tests passed in 48.272s; the focused native and entrypoint controls passed in 2.437s. Affected-package vet, formatting and diff checks passed. The native kubectl transport ran locally. The existing required hosted job supplies the pinned Helm 4.2 full-controller control; that optional local test was skipped because this host has a different Helm version.

Correctness and simplification review: the production change is one explicit manager option on the existing JSON Patch. No journal value, UID/resource-version precondition, deletion scope, policy, replay gate or retained-data path changes. The added transport fixture preserves the prune-protection annotation as well as the journal. This writer fix does not migrate an existing journal. Current terminal maintenance remains with #5042/#5044.

This author review satisfies no external review gate. Keep the PR draft until #5040 is merged, then bind current-head hosted checks and a substantive review before promotion.

> 🤖 Generated by the Agentic Engineer

Preserve both the dedicated journal manager regression and main's non-mutating admission DELETE request control.
Refs #5043; updates PR #5045.
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Integrated reviewed main 7120bf164af2eaf2c8734ef17b1f1e3be514e733 in signed head 91f3c2d8fdecd988e5599711a7dada8577ea5fa6. The sole merge conflict was between adjacent regressions; both the dedicated journal-owner check and main’s non-mutating admission DELETE-body check remain intact. The final diff against main is still four files, with the existing field-manager option and its documentation/tests.

Validation at this integrated tree: Go 1.26.9 affected-package race suite passed in 46.383 seconds, including the native kubectl journal transport; affected-package vet, diff checks and the existing PR-body contract passed. The separately pinned native Helm controller control is left to the required hosted job; the local run does not prove that controller path or production acceptance.

The PR remains draft, with no auto-merge or queue entry. New-head hosted checks and ordered semantic review are still required. This fixes ownership for future journals; it does not clear the existing journal or resolve the production recovery tracked in #5042.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Hosted validation is now verified at 91f3c2d8fdecd988e5599711a7dada8577ea5fa6 in CI run 39083652780, a successful pull-request run bound to this head.

The Detect Changes job completed the full Go helper suite, native retirement result classification, and stateless retirement preservation. Its preserved log shows checksum-pinned Helm 4.2.0, the mandatory native-control flag enabled, and the affected retirement race suite passing in 63.925 seconds. This closes the hosted native-controller validation gap recorded in the prior integration receipt.

This verifies the offline native control at this head. Current-head semantic review remains required; it does not establish production recovery or acceptance of a live journal change. The PR stays draft.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository YAML (base), Organization UI (inherited)
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 6c9d110d-b312-499a-af7d-9d837f7a3926
📥 Commits

Reviewing files that changed from the base of the PR and between 7120bf1 and a648b89.

📒 Files selected for processing: 4
  • scripts/retire-backstage-stateless/README.md
  • scripts/retire-backstage-stateless/fence.go
  • scripts/retire-backstage-stateless/fence_test.go
  • scripts/retire-backstage-stateless/native_test.go

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

📜 Recent review details
🔇 Additional comments: 4
scripts/retire-backstage-stateless/fence.go: 1

157-158: LGTM!

Also applies to: 175-175

scripts/retire-backstage-stateless/fence_test.go: 1

14-36: LGTM!

scripts/retire-backstage-stateless/native_test.go: 1

437-514: LGTM!

scripts/retire-backstage-stateless/README.md: 1

66-72: LGTM!

📝 Walkthrough

The guarded retirement journal writer now sends the namespace patch with the backstage-retirement field manager. Added tests check the field manager in execute mode and exercise the native kubectl request against a loopback API fixture. The native test verifies the UID and resource-version preconditions, retirement annotation, journal values, and successful fence installation. The README documents the field manager, existing journal handling, and regression coverage.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to a648b

New retirement journals will be written under a dedicated field manager, so the application reconciler no longer owns them. The patch payload and preconditions are unchanged. No actionable merge-blocking risk was found. Existing journals are not migrated, which is a documented limitation.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Issue #5043 requires a dedicated field manager for new retirement journals, native writer coverage, and preserved identity and payload protections. fence.go sends the existing JSON Patch with `--fie…
Out of Scope Changes check Passed The changes support #5043. The README documents the new field manager and regression coverage. The tests verify the writer, native kubectl transport, identity preconditions, and non-mutating controls.…
Title check Passed The title clearly and concisely identifies the main change: assigning new retirement journals an independent field owner.
Description check Passed The description directly explains the dedicated field manager, preserved protections, excluded cleanup work, linked issue, and merge-order requirement.
Full details: Linked Issues check

Explanation

Issue #5043 requires a dedicated field manager for new retirement journals, native writer coverage, and preserved identity and payload protections. fence.go sends the existing JSON Patch with --field-manager=backstage-retirement. The supplied change summary reports execute-mode writer coverage and native kubectl coverage. The current native tests verify server dry-run behavior, UID/resource-version preconditions, and journal binding. No evidence shows live replay or existing-journal maintenance.

Full details: Out of Scope Changes check

Explanation

The changes support #5043. The README documents the new field manager and regression coverage. The tests verify the writer, native kubectl transport, identity preconditions, and non-mutating controls. The evidence does not show terminal maintenance, live journal migration, retirement replay, or production recovery changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • 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

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

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Addressed the current-head CodeRabbit docstring warning in signed commit a648b89ceada6452b5a957a256b3046da0a16d75: the journal installation method now documents its controller, source and namespace checks, and the entrypoint regression documents the dedicated journal field manager it verifies. The change adds four comment lines across two files.

At this tree, the affected retirement package race suite passed in 84.440 seconds with a five-minute test deadline. Formatting and diff checks passed. A Go parser audit confirmed attached doc comments on all three named declarations changed by this PR, including the existing native kubectl regression.

Fresh hosted checks and semantic review are required for this new head. The PR stays draft; the earlier hosted controller evidence and review remain bound to the previous head. This does not clear an existing retirement journal or authorize a production replay. Recovery remains tracked in #5042.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Hosted validation is verified at a648b89ceada6452b5a957a256b3046da0a16d75 in successful CI run 39094906604. The run metadata independently binds this pull-request run to that exact head.

The Detect Changes job exercised the retirement race suite with checksum-verified Helm 4.2.0 and mandatory native controls enabled; it passed in 43.243 seconds. The complete current-head check collection contains 19 successes, 15 skips and no failed or pending checks.

The complete four-file diff preserves the journal identity checks and adds its independent field manager with native kubectl coverage. No further correctness or simplification change was identified in the author review. This receipt verifies offline behavior at the documentation-fix head; it does not establish production recovery. Current-head semantic review has been requested, and the PR remains draft.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Resolved by refutation: the Docstring Coverage warning in this review at a648b89ceada6452b5a957a256b3046da0a16d75 reports 75% from four analyzed functions but identifies no undocumented declaration.

A Go parser comparison of the complete base and head source finds exactly three changed named function declarations: installFence, TestJournalWriterUsesItsOwnFieldManager, and TestNativeJournalWriterKeepsItsManagerAndIdentityPreconditions. All three have attached documentation comments describing their behavior. The two new tests also contain three anonymous callbacks; those are local function literals, with their purpose described by the enclosing tests, rather than additional named declarations needing API doc comments. The parser audit fails if any changed named declaration lacks its comment or if the changed-declaration count differs from three. It passed with all three documented.

The reported fourth-function denominator does not establish a missing documentation change in this diff. This resolves that specific coverage warning; no file or check configuration was changed. The ordered review loop restarts at this same head, and the PR stays draft until the new review gate is satisfied. The completed CodeRabbit summary reports zero included reviews remaining, so its current allowance is retained as service state rather than spent on a redundant request.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-18T05:59:30.172453Z a648b89 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: a648b89cea

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Readiness verified at a648b89ceada6452b5a957a256b3046da0a16d75. The complete current-head collection has 19 successful checks, 15 skips and a successful CodeRabbit status. Codex completed its review with no findings in the exact-head receipt; complete conversation, review-object, inline-comment and thread reads contain no unresolved finding or maintainer direction. The earlier documentation warning has its specific same-head refutation recorded.

I independently read CI run 39094906604 and its successful Detect Changes job: checksum-pinned Helm 4.2.0 and mandatory native controls exercised the retirement race suite in 43.243 seconds. The complete four-file diff gives new journals their own field manager while retaining the UID, resource-version and journal-binding checks. No correctness or simplification issue was found in this handoff review.

The prerequisite #5040 is merged. This is ready for the normal merge queue at the evaluated head, behind the existing recovery entry. Existing-journal maintenance and production recovery remain #5042; this offline proof does not certify either.

@devantler
devantler marked this pull request as ready for review October 18, 2026 06:17
@devantler
devantler added this pull request to the merge queue Oct 18, 2026
@devantler
devantler removed this pull request from the merge queue due to a manual request Oct 18, 2026
@devantler devantler added blocked and removed blocked labels Oct 18, 2026
@devantler
devantler added this pull request to the merge queue Oct 18, 2026
Merged via the queue into main with commit 8de0904 Oct 19, 2026
35 checks passed
@devantler
devantler deleted the codex/backstage-journal-ownership-5043 branch October 19, 2026 00:47
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Give future Backstage retirement journals an independent field owner

1 participant