fix: summarize all actions in the fbc-operations complete state

[yashvardhannanavati]

The containerized fbc-operations handler reported only the index.db cleanup in its final state_reason:

The operator(s) ['X'] were successfully removed from the index image

That describes an intermediate step, not the outcome of the request, so a request that added fragments for X and Y read as though it had only removed X.

Report the cleanup as an in_progress update where it belongs, and compose the complete state from every action taken:

Successfully added operators ['X', 'Y'] to the index image
and removed operators ['X'] from the index.db

The removal clause is omitted when nothing needed removing. To make this possible, opm_registry_add_fbc_fragment_containerized() now also returns the operators added from the fragments, de-duplicated so a package carried by two fragments is not named twice.

[qodo-for-releng]

PR Summary by Qodo

Summarize all FBC operation actions in completion state

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Reports index.db cleanup as an in-progress operation.
• Summarizes added and removed operators in the final request state.
• De-duplicates fragment package names and covers both completion outcomes.

Diagram

sequenceDiagram
    participant H as Request Handler
    participant O as OPM Operations
    participant F as FBC Fragments
    participant D as index.db
    participant S as Request State
    H->>O: Add fragments
    O->>F: Extract packages
    F-->>O: Operator names
    O->>O: De-duplicate names
    O->>D: Verify conflicts
    D-->>O: Existing operators
    alt Conflicts found
        O->>S: Cleanup in progress
        O->>D: Remove conflicts
    end
    O-->>H: Added and removed
    H->>S: Complete summary

Loading

High-Level Assessment

The current approach is appropriate for this focused fix: the helper already discovers both added packages and database conflicts, so returning that information avoids duplicate fragment inspection in the handler. A dedicated result object could improve the growing tuple contract, but would introduce broader refactoring without materially improving this change.

Files changed (4) +142 / -15

Bug fix (2) +22 / -10

build_containerized_fbc_operations.pyCompose completion state from added and removed operators +8/-6

Compose completion state from added and removed operators

• Consumes the added-operator list returned by the OPM helper. The final state now always reports added operators and conditionally includes operators removed from index.db.

iib/workers/tasks/build_containerized_fbc_operations.py

opm_operations.pyReturn de-duplicated fragment operators and report cleanup progress +14/-4

Return de-duplicated fragment operators and report cleanup progress

• Extends the helper return contract with ordered, de-duplicated operator names extracted from all fragments. Moves index.db removal reporting into an in-progress state emitted immediately before cleanup.

iib/workers/tasks/opm_operations.py

Tests (2) +120 / -5

test_build_containerized_fbc_operations.pyCover complete state summaries for FBC operations +111/-4

Cover complete state summaries for FBC operations

• Updates mocks for the expanded helper return tuple and asserts the added-only completion message. Adds parameterized coverage for final summaries with and without index.db removals.

tests/test_workers/test_tasks/test_build_containerized_fbc_operations.py

test_opm_operations.pyVerify returned operators and cleanup progress states +9/-1

Verify returned operators and cleanup progress states

• Updates the expected helper result to include added operators. Verifies index.db cleanup emits an in-progress update only when conflicting operators exist.

tests/test_workers/test_tasks/test_opm_operations.py

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:53 AM UTC · Completed 6:07 AM UTC

Commit: ecbc65c · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $5.37

[qodo-for-releng]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

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 ↗

[fullsend-ai-review]

Risk Assessment: low (1/5)

Details

Small bug fix with 4 files and 159 lines, 50% test file ratio, no protected paths or CI/dependency changes, authored by an established contributor, with minimal recent churn and no revert history on the touched files.

Previous run

Risk Assessment: low (1/5)

Details

Low-risk bug fix: 4 files with 157 lines changed, strong test coverage (50% test file ratio), no protected paths or security-sensitive files touched, no CI or dependency changes, calm git history with minimal churn and single author, by a returning contributor with no linked issue.

[fullsend-ai-review]

Looks good to me

Previous run

Review

Findings

Low

• [missing-authorization] — No issue is linked to this PR. The change is non-trivial (142 changed lines across 4 files, including a public function signature change). While the PR body provides a clear and credible bug-fix rationale, linking to an issue establishes an authorization trail and makes the change discoverable from the tracker.

• [assertion-style-consistency] tests/test_workers/test_tasks/test_build_containerized_fbc_operations.py:179 — The assertion added at line 179 uses per-index access (mock_srs.call_args[0][2]) to match the pre-existing assertion at line 178, but the new dedicated test at line 751 asserts the full positional-args tuple (mock_srs.call_args[0] == (request_id, 'complete', expected_state_reason)). Within the same PR, two different assertion idioms are used for the same mock. The tuple comparison is the more thorough approach since it verifies all three positional arguments in a single statement.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:27 AM UTC · Completed 3:41 AM UTC

Commit: 504db40 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.85

Superseded by updated review

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 6:37 AM UTC · Completed 6:50 AM UTC

Commit: 504db40 · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.19

[fullsend-ai-retro]

Retro: PR #1391 — fix: summarize all actions in the fbc-operations complete state

Timeline

1. 2026-09-14 22:59 — Issue #1389 opened: containerized fbc-operations handler reports incorrect state_reason.
2. 2026-09-15 05:52 — PR #1391 opened by @yashvardhannanavati (co-authored with Claude Opus 5). Branch: fbc_operations_reason.
3. 2026-09-15 05:52–06:07 — First review agent run (34934498065): APPROVE, risk low (1/5), 2 low findings (missing linked issue, assertion style inconsistency) both marked non-actionable. Cost: $5.37.
4. 2026-09-15 06:49 — Human reviewer @lipoja approved (first commit).
5. 2026-09-16 03:26 — Force-push (rebase/squash to single commit 504db40).
6. 2026-09-16 03:27–03:41 — Second review agent run (35051807860): Prior review discarded (REVIEW_APP_CLIENT_ID empty → unverifiable-wrong-app). Full re-review from scratch: APPROVE, no findings above threshold. Cost: $3.85.
7. 2026-09-16 06:31 — Human reviewer @lipoja re-approved (second commit).
8. 2026-09-16 06:36 — PR merged.

Assessment

Review quality: Good. The review agent correctly identified this as a well-scoped, low-risk bug fix with thorough test coverage. Its two low-severity findings on the first commit (missing linked issue, assertion style inconsistency) were reasonable observations correctly marked non-actionable. The challenger pass on the second review correctly identified that the "missing linked issue" finding was unsupported — there is no documented policy requiring issue links in this repo, and many recent commits lack them. The human reviewer approved without comments on both commits, consistent with the agent's assessment.

Rework: None. The force-push was a rebase/squash, not a response to review feedback. Zero iterations were needed.

Token cost: $9.22 total. The redundant full re-review ($3.85) was caused by the known REVIEW_APP_CLIENT_ID configuration gap — the harness could not verify the first review's provenance after force-push, so the second run performed a complete review from scratch rather than an incremental delta review.

Autonomy: High alignment. The review agent's verdict matched the human reviewer's on both commits. No findings were missed by the agent that the human caught. For this class of change (well-scoped bug fix with comprehensive tests), the agent demonstrated reliable judgment.

Existing issues — new evidence

• Review provenance chain discards own prior review due to empty expected app client_id fullsend-ai/fullsend#7338 and #6911: This PR provides another concrete data point for the empty REVIEW_APP_CLIENT_ID in per-repo install mode. The second review cost $3.85 that would have been avoided (or reduced to a delta review) if provenance validation had succeeded. Agents repo: fullsend-ai/agents@v0.43.0.
• Configure REVIEW_APP_CLIENT_ID so provenance validation works on re-reviews fullsend-ai/agents#931 and Parse bundle version when propagating bundles in merge-index-image #203: Same root cause from the agents-repo configuration side.
• containerized fbc-operations handler on main branch has incorrect state_reason for complete state #1389: This issue describes the exact bug that PR fix: summarize all actions in the fbc-operations complete state #1391 fixes, but the PR does not include closing keywords (Fixes #1389). The issue remains open after merge.

No new proposals

The workflow performed well overall. The only significant inefficiency (redundant full re-review on force-push due to empty REVIEW_APP_CLIENT_ID) is already tracked by multiple open issues across fullsend-ai/fullsend (#7338, #6911) and fullsend-ai/agents (#931, #203). Recently closed issues (#5388, #6442) attempted partial fixes but the problem persists for per-repo install mode, as evidenced by this PR.