Skip to content

docs: update per-org removal plan for immediate-removal decision - #5995

Closed
ralphbean wants to merge 2 commits into
mainfrom
docs/remove-org-mode-plan-update-immediate-removal
Closed

docs: update per-org removal plan for immediate-removal decision#5995
ralphbean wants to merge 2 commits into
mainfrom
docs/remove-org-mode-plan-update-immediate-removal

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • The team decided to skip the phased deprecation (warnings + migration command) and remove per-org support immediately, in a single effort (#2302, meeting update 2026-08-06), rather than ADR-0044's original two-phase rollout.
  • Drops the Phase 1 PRs (deprecation warnings, docs, migration command, migration e2e test — formerly PRs 1-5) from docs/plans/deprecate-per-org-install.md, and renumbers the removal-only PRs (formerly Phase 2, PRs 6-15) to 1-10.
  • Updates the dependency graph, context section, and release checklist to match. Cross-references #5990 for the e2e admin suite conversion, which is tracked separately.

Note: ADR-0044 itself still documents the original phased decision — this PR only updates the implementation plan doc, not the ADR (per this repo's convention that ADR decision sections aren't rewritten in place). Whether ADR-0044 needs a formal update/supersession is a separate question worth raising with the team.

Test plan

  • make lint-md-links passes
  • Pre-commit hooks pass (gitlint, markdown link checks, etc.)
  • Reviewer confirms PR renumbering and dropped Phase 1 content accurately reflect the team's decision on Remove per-org install mode #2302

The team decided to skip the phased deprecation (warnings + migration
command) and remove per-org support immediately and completely in a
single effort (#2302, meeting update 2026-08-06), rather than the
two-phase rollout ADR-0044 originally described.

Drop the Phase 1 deprecation-warning/migration-command PRs (formerly
PRs 1-5) from the implementation plan and renumber the removal-only
PRs (formerly Phase 2, PRs 6-15) to 1-10. Update the dependency graph,
context, and release checklist to match. Cross-reference #5990 for the
e2e suite conversion tracked separately.

ADR-0044 itself still documents the original phased decision; see #2302
for the updated decision context.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean requested a review from a team as a code owner August 6, 2026 19:54
@ralphbean ralphbean added the fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs label Aug 6, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Docs: revise per-org removal plan to reflect immediate removal decision

📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Document the decision to remove per-org support immediately (skipping phased deprecation).
• Remove Phase 1 items from the plan and renumber the removal PR sequence to 1–10.
• Update dependency graph and release checklist; reference #5990 for e2e suite conversion.
Diagram

graph TD
  D{"#2302 decision"} --> P["docs/plans deprecate plan"] --> W["Removal PRs 1-10"]
  A["ADR-0044"] --> P
  P --> E(["#5990 e2e conversion"])

  subgraph Legend
    direction LR
    _dec{"Decision"} ~~~ _doc["Doc"] ~~~ _issue(["Issue"])
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Supersede ADR-0044 with a new ADR
  • ➕ Creates a canonical, versioned record of the updated decision
  • ➕ Avoids long-lived divergence between ADR decision text and implementation plan
  • ➖ More process overhead (review, signoff) than a plan update
  • ➖ May slow down execution if treated as a gate
2. Amend ADR-0044 with an explicit addendum section
  • ➕ Keeps the decision update co-located with the original ADR
  • ➕ Lower overhead than writing a new ADR
  • ➖ Conflicts with the stated convention of not rewriting ADR decision sections in place
  • ➖ Can blur historical record if not clearly structured as an addendum
3. Split into two docs: 'Original phased plan (archived)' and 'Immediate removal plan'
  • ➕ Eliminates ambiguity for readers; each doc has a single coherent narrative
  • ➕ Keeps historical phased plan accessible without mixing states
  • ➖ More documentation surface area to maintain
  • ➖ Readers may still land on the wrong doc without strong cross-linking

Recommendation: The PR’s approach (update the implementation plan doc while leaving ADR-0044 unchanged) matches the stated repo convention and minimizes churn. The main follow-up worth considering is a lightweight formalization of the decision change (new ADR or addendum) so the canonical ADR set doesn’t permanently diverge from execution reality; this PR can proceed independently, but capturing the decision in the ADR system will reduce future confusion.

Files changed (1) +62 / -265

Documentation (1) +62 / -265
deprecate-per-org-install.mdRewrite per-org deprecation plan as immediate removal-only roadmap +62/-265

Rewrite per-org deprecation plan as immediate removal-only roadmap

• Replaces the phased deprecation narrative with the updated #2302 decision to remove per-org support immediately. Drops Phase 1 PRs (warnings/docs/migration tooling/tests), renumbers the remaining removal PRs to 1–10, and updates the dependency graph and release checklist accordingly. Adds/clarifies cross-references to #5990 for e2e/admin suite conversion work tracked separately.

docs/plans/deprecate-per-org-install.md

@qodo-code-review

qodo-code-review Bot commented Aug 6, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Misleading plan title ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The plan is now explicitly removal-only (deprecation/migration phase dropped), but the document’s H1
still says “Deprecate”, which can mislead readers about the scope and intent of the plan. This
increases the chance someone expects a phased deprecation or looks in the wrong place for the
immediate-removal plan.
Code

docs/plans/deprecate-per-org-install.md[74]

+## Removal plan
Relevance

●●● Strong

Team commonly accepts small doc-consistency fixes; misleading headings/wording are routinely
corrected.

PR-#5273
PR-#5941

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The H1 remains “Deprecate…”, while the updated context explicitly says the team will skip the
deprecation period and the plan “covers only the removal work”, and the main section header is now
“Removal plan”.

docs/plans/deprecate-per-org-install.md[1-1]
docs/plans/deprecate-per-org-install.md[15-25]
docs/plans/deprecate-per-org-install.md[74-76]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The document’s top-level title still frames this as a deprecation plan, but the edited content now describes an immediate, removal-only effort. This mismatch makes the plan easier to misinterpret.

### Issue Context
The PR changes the plan from phased deprecation/removal to removal-only (skipping deprecation warnings + migration command). The H1 should match that scope.

### Fix
Update the H1 to reflect the removal-only plan (e.g., “Implementation Plan: Remove Per-Org Installation Mode (ADR-0044)”, or similar wording consistent with team terminology).

### Fix Focus Areas
- docs/plans/deprecate-per-org-install.md[1-1]
- docs/plans/deprecate-per-org-install.md[15-25]
- docs/plans/deprecate-per-org-install.md[74-76]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 54 rules

Grey Divider

Tip of the day
💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread docs/plans/deprecate-per-org-install.md

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review-only pass (not self-assigning, approving, or requesting changes). Left 3 inline comments on docs/plans/deprecate-per-org-install.md. One additional finding below is on a file this PR doesn't touch, so it can't be anchored inline:

MEDIUM: ADR-0044 §2 "Implementation plan" now cross-references a stale, mismatched PR list (docs/ADRs/0044-deprecate-per-org-installation-mode.md, §2, around line 352)

This PR touches only docs/plans/deprecate-per-org-install.md; ADR-0044 is untouched. ADR-0044 §2 still says the companion plan doc "contains a 15-PR dependency graph" and enumerates "Phase 1 (5 PRs)" plus "Phase 2 (10 PRs)" numbered 6-15 (e.g. "PR 10: Remove org-level forge methods", "PR 15: Remove per-org e2e tests..."). This PR renumbers the plan doc to a flat 1-10 scheme and deletes Phase 1 entirely (removed "### PR 2: Update documentation", "### PR 3: Add per-repo e2e test", the Phase 1/Phase 2 headers, and the old dependency graph). The ADR's detailed, number-matched PR list is now factually wrong versus the plan doc it explicitly points readers to, not just stale prose in the Decision section (which the PR body's disclaimer does address). A reader following ADR-0044 §2's numbering into the plan doc will find PR numbers/content that no longer correspond (e.g., ADR's "PR 10: forge methods" vs. plan doc's "PR 5: forge methods").

Suggestion: add a one-line annotation under ADR-0044 §2 (using the ADR's own stated convention for minor cross-reference annotations) noting the detailed PR list has been superseded by the renumbered, Phase-1-dropped plan in docs/plans/deprecate-per-org-install.md, so readers don't cross-reference stale PR numbers between the two documents. Separately, since ADR-0044's Decision section materially changed (two-phase rollout -> immediate single-phase removal), consider opening a tracking issue for a formal ADR supersession/amendment rather than leaving that as an open question only in the PR body.

Comment thread docs/plans/deprecate-per-org-install.md
Comment thread docs/plans/deprecate-per-org-install.md Outdated
Comment thread docs/plans/deprecate-per-org-install.md
- Retitle the plan doc's H1 to match the removal-only scope (was
  still "Deprecate...")
- Add manual-migration guidance for existing per-org installs, since
  dropping the migrate command leaves no automated path
- Narrow the #5990 e2e-conversion caveat to the lifecycle test only;
  clarify lock_test.go/cleanup.go proceed independently
- Add an explicit docs-update task to PR 10 covering org-mode.md,
  architecture.md, and a guides sweep
- Annotate ADR-0044 §2 as superseded by the plan doc's renumbering

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 10, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:24 PM UTC · Completed 4:43 PM UTC

Commit: d10acf1 · View workflow run →

@ralphbean

Copy link
Copy Markdown
Member Author

Re: #5995 (review)

Good point. Pushed d10acf1 — added an update note under ADR-0044 §2 flagging it as superseded by the plan doc's renumbering, following the same pattern as ADR-0067's update note. Didn't open a tracking issue for a formal ADR supersession — that feels like a separate conversation to have with the team, happy to file one if it'd help.

@ralphbean

Copy link
Copy Markdown
Member Author

Re: #5995 (comment)

Same title issue flagged inline — fixed in d10acf1.

@ralphbean

Copy link
Copy Markdown
Member Author

Closing this one. While it was open, #5972 deleted docs/plans/ entirely and gitignored it going forward — so the file this PR updates doesn't belong in the tree anymore, and rebasing to keep it alive would just fight that decision.

#2302 stays open — the actual per-org removal hasn't happened, we just lost the place to track a per-file implementation plan for it. Whoever picks it up next will want a different vehicle for the checklist, maybe a tracking issue with subtasks instead of a markdown plan.

@ralphbean ralphbean closed this Aug 10, 2026
@ralphbean
ralphbean deleted the docs/remove-org-mode-plan-update-immediate-removal branch August 10, 2026 16:29
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 10, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 4:31 PM UTC · Completed 5:18 PM UTC

Commit: d10acf1 · View workflow run →

@fullsend-ai-review

Copy link
Copy Markdown

Review skipped — this PR is already closed.

The /fs-review command only reviews open pull requests.

Posted by fullsend post-review check

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5995 — docs: update per-org removal plan

Agents repo discovered: fullsend-ai/agents (from review run 31408724683 logs).

Timeline

  1. Aug 6 19:54 UTC — PR #5995 opened by ralphbean, updating docs/plans/deprecate-per-org-install.md to reflect the team's decision to skip phased deprecation and remove per-org support immediately (#2302). Qodo bot reviewed within 3 minutes, catching 1 optional finding (misleading H1 title). No fullsend review agent dispatch occurred — the pull_request_target/opened webhook was never delivered to GitHub Actions (confirmed by checking all 509 fullsend.yaml runs on Aug 6; a 4.5-hour gap in pull_request_target runs spans the PR creation time).
  2. Aug 7 12:51 UTC — PR #5972 merged, deleting docs/plans/ entirely and gitignoring it.
  3. Aug 7 13:34 UTC — Human reviewer waynesun09 posted 4 substantive MEDIUM findings: (a) no manual-migration guidance for existing per-org installs now that admin migrate is dropped, (b) inconsistent scoping of the e2e: convert admin install/uninstall suite from org mode to per-repo mode #5990 e2e-conversion caveat across the table and PR-10 detail, (c) dropped user-facing/architecture doc-update task with no replacement PR, (d) ADR-0044 section 2 now references a stale 15-PR list.
  4. Aug 10 16:23 UTC — ralphbean pushed commit 2 addressing all 4 human review findings. The synchronize event successfully dispatched the review agent (run 31408724683), confirming the dispatch infrastructure works for this author/repo.
  5. Aug 10 16:29 UTC — ralphbean closed the PR without merge, recognizing that chore(docs): delete plans and superpower stuff #5972 had already deleted the target file.
  6. Aug 10 16:43 UTC — Review agent completed after ~19 minutes (4 sub-agents: code-review, docs-review, pr-review, issue-labels). Post-script correctly detected the PR was closed and skipped posting. Retro dispatched on close.

Observations

Silently dropped webhook (novel finding): The pull_request_target/opened event on Aug 6 was never delivered to GitHub Actions. No workflow run was created — this is distinct from #1919 (startup_failure where a run exists but dies) and #2711 (infra failures where runs exist but fail). Those issues address failure modes where a run record exists; here, no run was ever created. No existing issue proposes a detection or recovery mechanism for silently lost webhooks. Recent merged PRs in this repo (#6022, #6019, #6016, #6015, #6011) all received review bot comments, so this appears to be an isolated event rather than a systematic routing gap. One proposal below.

Evidence for existing issues (no new proposals warranted):

  • #2388 (cancel in-flight review when PR closes): The review agent continued for ~14 minutes after the PR closed at 16:29, completing at ~16:43. ~19 total minutes of compute produced findings that were never surfaced.
  • #5375 / #5619 (skip retro for unmerged PRs with no/minimal agent interaction): This retro itself is an example — running on a closed, unmerged PR where the only agent run was a review whose findings were discarded.
  • #5817 (consolidate 22+ dispatch-skip issues): This PR adds another data point to the pattern of wasted dispatch compute.

Autonomy readiness

No meaningful agent-vs-human comparison is possible. The human reviewer (waynesun09) reviewed commit 1 and found 4 real issues. The review agent only ran on commit 2 (which incorporated the human's fixes) and never posted its findings. The review agent's run logs show it analyzed ADR cross-reference staleness and phase-skip intent coherence — suggesting it may have caught similar issues on the original content — but this cannot be validated.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fullsend-fix Enables automatic bot-triggered fix runs on human-authored PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants