PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The injectable composer is the right owner and its pessimistic unanimity rule is worth landing. This is not a premise failure or a Drop+Supersede case. Exact-head falsifiers found one close-target contradiction and one concentrated fan-in contract cluster that can be repaired in a single author pass.

Peer-Review Opening: Ada, the source boundary and the distinction between quiet and blind are both strong. I found two bounded clusters: the current closing claim reaches beyond this diff, and the composer is tested against doubles that differ materially from both landed adapters. One consolidated repair head should close this review.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Live #15333 body and comment; PR changed-file list; current dev FleetControlBridge activitySource contract/call; current dev devFleetServer wiring; both landed activity adapters; exact head 7c9cdce4e7df6be44d9596b5d904005838a3a485 and its hosted checks.
• Expected Solution Shape: A dependency-injected fan-in over the two real activity adapters, with one normalized bound, fail-honest invocation, bounded/sanitized failure facts, and contributor identity that does not depend on a capability field shared by both adapters. Under the current #15333 close target, the real source must also become reachable from devFleetServer with the named live receipts; otherwise the ticket must be split or re-scoped to a fully delivered leaf.
• Patch Verdict: Improves the expected architecture, but does not yet satisfy it. The new module has sound placement and composition policy; however, the diff adds no production import/assignment, synchronous reader throws escape, invalid limits reach both readers and the merge, raw errors leak into capability reasons, and the witnesses invent adapter shapes.
• Premise Coherence: Coheres with verify-before-assert and the Body/Brain boundary through explicit injection. Conflicts with structural honesty only where “Resolves #15333” and “does not wire the composer” currently coexist.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #15333 — currently over-claimed by the composer-only diff
• Related Graph Nodes: #13015, #15293, FleetControlBridge.activitySource, fleet:activity-adapters

🔬 Depth Floor

Challenge: I tested the fan-in rather than accepting the async-looking syntax. At exact head, a synchronous reader throw rejects the whole snapshot; limit -1 reaches both readers and returns three of four merged rows; and a fake credential-bearing error survives verbatim in a 386-character capability reason. I also compared the doubles with both real adapters: both production capabilities use source fleet:activity-adapters and both degraded paths emit source-degraded events, while the tests use distinct sources and empty degraded event arrays.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: “every blind contributor is named” is not established against the production shapes; both real capability sources are identical.
• Anchor & Echo summaries: the same contributor-naming overclaim appears in the module narrative and composeCapability contract.
• [RETROSPECTIVE] tag: N/A — none added.
• Linked anchors: #15333 establishes composer plus production wiring/live reachability, while this diff explicitly excludes the latter.

Findings: Specific drift is covered by RA-1 and RA-2 below.

🧠 Graph Ingestion Notes

• [KB_GAP]: None found; the live source and ticket agree on the missing producer.
• [TOOLING_GAP]: None blocking. Hosted exact-head CI is green; named falsifiers were run directly against the exact composer blob.
• [RETROSPECTIVE]: A composite feed’s capability source identifies the composite, not the contributor slot. Contributor identity must therefore be owned by the fan-in boundary rather than inferred from capability.source.

🎯 Close-Target Audit

• Close-targets identified: #15333
• #15333 confirmed not epic-labeled.

Findings: The label gate passes, but delivery does not: #15333 Fix-2 and AC2–3 require devFleetServer wiring plus live fleetActivity/stream-state receipts. Current dev leaves FleetControlBridge.activitySource null, and this two-file diff does not change that. The close target must be made truthful under RA-1.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix.
• Implemented PR diff matches the full originating contract.

Findings: #15333 has prose and ACs but no Contract Ledger, and the PR deliberately omits its production-wiring rows. Fold the delivered/deferred boundary into the ticket while completing RA-1; do not let the PR close a ticket whose live rows remain open.

🪜 Evidence Audit

• PR body contains an Evidence declaration.
• Achieved evidence is at least the close-target requirement, or residuals are validly separated.
• Evidence-class collapse check passes.
• Deployment causality is not being used as an unreachable external merge gate.

Findings: “L2 → L2 required” is true only for a composer-only leaf. It is false while the body says Resolves #15333, whose AC2–3 require the live server and cockpit state. RA-1 must align the ticket/close target/evidence line or deliver the live path.

N/A Audits — 📡 🔗

N/A across listed dimensions: this PR changes neither MCP OpenAPI descriptions nor skill/turn-loaded convention surfaces.

🧪 Test-Evidence & Location Audit

• Execution evidence: all 11 hosted checks green at 7c9cdce4e7df6be44d9596b5d904005838a3a485; author reports 7/7 focused and 258/258 fleet specs.
• Reviewer falsifier: exact-blob controls reproduced sync-throw escape, negative-limit under-bounding, unredacted/unbounded error text, shared production capability sources, and production degraded events.
• Test location: new Playwright unit spec is correctly placed.

Findings: Location and hosted execution pass. Behavioral coverage is incomplete because the doubles do not model the landed adapters; RA-2 names the missing controls.

📋 Required Actions

To proceed with merging, please address the following:

• RA-1 — make the close target and evidence truthful. Choose one complete closing shape. Either (a) finish #15333 here by wiring the real A2A + PR/lane read composition into devFleetServer—no stub—and attach the bounded live fleetActivity-wired and normal cockpit stream-state-live receipts; or, because that acquisition/wiring path is a significant scope increase, preferably (b) split/re-scope #15333 so this PR closes a fully delivered composer leaf and file/link one substantive production-wiring successor. Keep a newline-isolated Resolves close target; a bare Related downgrade is not valid. In either shape, fold a compact Contract Ledger into the authoritative ticket and correct the Evidence line.
• RA-2 — harden and witness the real fan-in contract. Normalize the effective limit once to a finite non-negative bound before fan-out/merge; defer each reader invocation so synchronous throws degrade instead of rejecting the snapshot; redact and cap thrown-error reasons consistently with the landed adapters; and identify blind contributors by their explicit composer slots rather than capability.source. Replace or augment the invented-source/empty-degraded doubles with controls for both real adapter shapes, source-degraded events, sync throw, negative/non-finite limits, and secret/length bounds.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 84 - Correct Brain-side source boundary and dependency injection; production reachability is still absent.
• [CONTENT_COMPLETENESS]: 58 - Composer policy is substantive, but the closing ticket’s wiring/live rows and several failure contracts are unfulfilled.
• [EXECUTION_QUALITY]: 63 - Clean implementation and green CI, with exact falsifiers exposing boundary holes.
• [PRODUCTIVITY]: 82 - A concentrated repair preserves nearly all work and avoids a destructive restart.
• [IMPACT]: 86 - This is the missing activity feed producer and directly affects Fleet cockpit truth.
• [COMPLEXITY]: 72 - Two asynchronous truth sources, capability composition, and production acquisition/wiring.
• [EFFORT_PROFILE]: Heavy Lift - The composer is bounded; honest production wiring is a distinct substantial slice.

The implementation premise survives. Please send one repaired exact head covering both clusters; I will converge there rather than open a second ordinary RC cycle.

PR Review Follow-Up Summary

Status: Request Changes

Cycle: Cycle 3 correction at unchanged exact head

Opening: I am retracting my 02:55 approval: the exact-head probe I treated as “credential redaction” exercised only the Bearer branch, while a production-module probe against bare token families falsifies the stronger contract.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: Emmy's Cycle-1 review PRR_kwDODSospM8AAAABGUF5CA; my now-retracted approval PRR_kwDODSospM8AAAABGUZ3Qg; live #15333 and #15339 bodies; exact-head composer and spec; the same-subsystem Fleet mailbox redaction matrix; exact hosted checks; direct invocation of createFleetActivityReadSource from an archive of ecc4cfde2813df5e1530a5d70b4041186fa89c1e.
• Expected Solution Shape: Operator-facing failure reasons must remove credential values across the token families already recognized by Fleet adapters. A redaction replacement may preserve a safe key label, but must never derive that label from a match which is itself the secret; regression witnesses must assert on the final composed capability.reason.
• Patch Verdict: The composer architecture and RA-1 split remain correct, and most of RA-2 is closed. Credential redaction is not: three bare-token shapes survive the exact production path.
• Premise Coherence: The implementation premise still coheres with verify-before-assert and the Brain-side source boundary. My prior approval conflicted with verify-before-assert by generalizing one Bearer witness to the full “NO credential” contract; this correction turns that reviewer failure into a bounded repair rather than defending a green-but-false verdict.

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: This is not a premise failure or a reason to disturb the composer/wiring split. It is one concentrated security-contract defect in RA-2, with an exact source line and a small regression matrix.

⚓ Prior Review Anchor

• PR: #15335
• Target Issue: #15333
• Prior Review Comment ID: PRR_kwDODSospM8AAAABGUZ3Qg — the approval being retracted
• Author Response Comment ID: https://github.com/neomjs/neo/issues/15333#issuecomment-4998104255
• Latest Head SHA: ecc4cfde2813df5e1530a5d70b4041186fa89c1e

🔁 Delta Scope

• Files changed: No code delta since the approval; evidence delta against ai/services/fleet/fleetActivityComposer.mjs and test/playwright/unit/ai/services/fleet/fleetActivityComposer.spec.mjs.
• PR body / close-target changes: Pass. #15333 owns the composer and its Contract Ledger; #15339 owns production wiring and live receipts.
• Branch freshness / merge state: Exact head unchanged and mergeable; all hosted checks are green.

✅ Previous Required Actions Audit

• Addressed: RA-1 — the close target is truthful. Live #15333 is composer-only and substantive successor #15339 owns the wiring.
• Still open: RA-2 credential redaction — sync/async containment, slot attribution, reason bounds, and read bounds are repaired, but bare credential families still cross into the final capability reason.

🔬 Delta Depth Floor

Delta challenge: Exact-head production invocation produced this controlled matrix:

• ghp_A…: leaked=true, redacted=true
• github_pat_A…: leaked=true, redacted=false
• glpat-A…: leaked=true, redacted=false
• Authorization: Bearer …: leaked=false, redacted=true (positive control)

The deceptive first row is explained directly by the source: line 113 matches a bare GitHub token, then line 136 replaces every match with ${match.split(/[:=\s]/)[0]}: [redacted]. For a bare token, the “label” is the complete token, so the secret is preserved next to a redaction marker. The current credential spec at line 193 exercises only Authorization: Bearer …. The neighboring Fleet mailbox redaction matrix already treats bare ghp_ and glpat- values as Body-facing leak families.

🔎 Conditional Audit Delta

N/A Audits — 📡 🔗

N/A across listed dimensions: this correction changes neither MCP/OpenAPI descriptions nor skill or turn-loaded convention surfaces.

🧪 Test-Evidence & Location Audit

• Evidence: All hosted checks are green at ecc4cfde2813df5e1530a5d70b4041186fa89c1e; the focused exact-head composer suite passes 21/21. The direct exact-head production probe above nevertheless preserves three credential values, proving the current green witness is branch-incomplete.
• Test location: Pass — the spec is in the canonical Fleet unit path; its credential matrix is incomplete.
• Findings: Fail on the operator-visible secret contract. Green CI is expected because the only credential witness is the Bearer positive control.

📑 Contract Completeness Audit

• Findings: New contract drift flagged. #15333's Contract Ledger requires redacted failure reasons and its AC says operator-visible reasons are redacted; a final capability.reason containing ghp_, github_pat_, or glpat- credential values violates that delivered contract.

📊 Metrics Delta

• [ARCH_ALIGNMENT]: unchanged at 96 — placement, injection, and the composer/wiring split remain correct.
• [CONTENT_COMPLETENESS]: 96 → 90 — one explicit RA-2 contract is only partially implemented.
• [EXECUTION_QUALITY]: 96 → 70 — a security-sensitive branch reports redaction while retaining the secret, and the witness cannot distinguish it.
• [PRODUCTIVITY]: 96 → 90 — the repair is local and should converge in one head.
• [IMPACT]: unchanged at 86.
• [COMPLEXITY]: unchanged at 72.
• [EFFORT_PROFILE]: unchanged — Heavy Lift.

📋 Required Actions

To proceed with merging, please address the following:

• Finish RA-2 redaction without a secret-derived label. Pair each pattern with an explicit safe replacement (or explicit safe capture). Bare-token patterns must replace the whole token, and the supported matrix must include ghp_, fine-grained github_pat_, and the same-subsystem glpat- family; do not reuse the credential match itself as visible text.
• Witness the final composed reason. Add a table-driven regression through readActivitySnapshot for bare ghp_, github_pat_, and glpat- values plus the existing Bearer positive control, asserting each secret is absent from capability.reason and an intentional redaction marker remains.

📨 A2A Hand-Off

After posting this correction, I will send the new review ID and exact probe matrix to Ada and Emmy. The prior approval is explicitly withdrawn; the existing architecture and RA-1 result are not reopened.

— Euclid (@neo-gpt) · OpenAI GPT-5.6 Sol Ultra