PR Review Summary

Status: Request Changes

🪜 Strategic-Fit Decision

Per §9 Strategic-Fit Step-Back:

• Decision: Request Changes
• Rationale: The deletion premise is correct and should not be superseded again: #13349 is closed as the wrong constrained-blueprint direction, and #14826 is the explicit retraction ticket. This is not Drop+Supersede because the patch mostly executes the right damage-control shape. It is still not approvable because the changed childapp-connect proof fails at exact head, contradicting the PR evidence claim that the capability coverage survived.

This is the right teardown after the car-versus-toaster miss: delete the brittle create spine, keep only the bare popup host the multi-window path still needs. I cannot approve it yet because the replacement NeuralLinkChildappConnect fixture is red locally before it reaches the Neural Link assertions.

🧭 Patch-Blind Premise Snapshot

• Inputs Read Before Patch: #14826 body + amendment comment, #13349 supersession comments including the 2026-07-05 operator/product ruling, changed-file list, live PR state at f5e81c857fd8fd91a8455988bc9a102f5bcf8926, hosted checks, origin/dev...HEAD diff, and stale-reference greps over apps/, test/, and src/.
• Expected Solution Shape: Remove the landed constrained-blueprint create module and first-widget grammar/materializer/evidence code. Preserve only the bare apps/agentos/childapps/widget popup host required by openComponentWindow / dashboard popupUrl, and preserve Neural Link childapp-connect coverage by moving its fixture to a non-funnel childapp.
• Patch Verdict: Mostly matches: the create module is gone, funnel utility/view/spec files are gone, and the only agentos/childapps/widget code references are the restored bare popup-host route/config/ignore exceptions/dashboard popupUrls. Execution contradicts the claimed coverage because the modified NeuralLinkChildappConnect test times out on the colors fixture.
• Premise Coherence: Coheres with verify-before-assert and friction→gold: it removes wrong-shape substrate instead of preserving sunk-cost code. The review blocker is execution evidence, not a premise objection.

🕸️ Context & Graph Linking

• Target Epic / Issue ID: Resolves #14826
• Related Graph Nodes: #13349, #13445, #14783, apps/agentos/view/create, apps/agentos/childapps/widget, NeuralLinkChildappConnect

🔬 Depth Floor

Challenge OR documented search (per guide §7.1):

Challenge: The replacement childapp-connect proof waits for .neo-viewport to be visible, but the empty colors childapp mounts #colors-widget-viewport as a hidden viewport. The exact-head run never reaches connectToApp('colors') or the component readback, so the PR currently removes the old funnel proof without a passing replacement proof.

Rhetorical-Drift Audit (per guide §7.4):

• PR description: framing matches the deletion shape
• Anchor & Echo summaries: the new bare-host JSDoc is scoped to popup host behavior
• [RETROSPECTIVE] tag: N/A
• Linked anchors: #13349 / #14826 substantiate the supersession/removal direction

Findings: Drift flagged: the PR body claims NeuralLinkChildappConnect is green, but exact-head local e2e falsifies that claim.

🧠 Graph Ingestion Notes

• [KB_GAP]: N/A
• [TOOLING_GAP]: Local e2e needed an isolated :8087 config because the standard non-CI e2e config would reuse an unrelated :8080 server from /Users/Shared/github/neomjs/neo. Sandboxed Chrome launch failed with EPERM; rerun outside the sandbox reached test execution.
• [RETROSPECTIVE]: This PR is productive damage control: the correct mergeable shape is deletion of the constrained blueprint spine plus preservation of the actual multi-window host primitive.

🎯 Close-Target Audit

• Close-targets identified: #14826
• #14826 confirmed not epic-labeled

Findings: Pass.

📑 Contract Completeness Audit

• Originating ticket contains a Contract Ledger matrix
• Implemented PR diff matches the Contract Ledger exactly

Findings: Scope mostly matches the amended ledger, including the restored bare popup host. The evidence-preservation clause is not satisfied until NeuralLinkChildappConnect.spec.mjs passes on the replacement fixture.

🪜 Evidence Audit

• PR body contains an Evidence: declaration line
• Achieved evidence ≥ close-target required evidence
• If residuals exist: N/A, no residuals declared
• Two-ceiling distinction: PR explains the local :8080 contamination risk
• Evidence-class collapse check: current review language cannot accept the claimed L2 e2e evidence while a named changed e2e fails locally

Findings: Evidence mismatch flagged.

N/A Audits — 📡 🔗

N/A across listed dimensions: this PR does not touch MCP OpenAPI descriptions or skill/convention surfaces.

🧪 Test-Execution & Location Audit

• Branch checked out locally: temporary exact-head worktree at f5e81c857fd8fd91a8455988bc9a102f5bcf8926
• Canonical Location: changed e2e files remain under test/playwright/e2e/
• If a test file changed: ran the specific changed e2e coverage locally
• If code changed: verified stale references and syntax checks

Findings: Test failure flagged in Required Actions.

Evidence run:

• gh pr checks 14828 --repo neomjs/neo — all hosted checks pass, including unit; no hosted e2e check is present in the rollup.
• git diff --check origin/dev...HEAD — pass.
• git grep -n "view/create" -- apps test src — no matches.
• git grep -n "CreateSurface|CreationStateProvider|CreatedInstances|CreatedInstance|acceptPath|blueprintSchema|creationFlowState|requestRoute|EvidencePane|RequestIntake|parseEditRequest|validateRequest|firstWidgetEditModel|blueprintEvidence|createdGridEvidence" -- apps test src — only design/resource mirror prose, no live code/test in-edges.
• node --check on the modified surviving .mjs files — pass.
• Exact-head e2e, isolated :8087, escalated after sandbox Chrome EPERM:
  • NeuralLinkCreateGrid.spec.mjs — pass.
  • NeuralLinkWindowOps.spec.mjs — pass.
  • NeuralLinkChildappConnect.spec.mjs — fail: page.waitForSelector('.neo-viewport', {state: 'visible'}) times out while #colors-widget-viewport.neo-viewport is attached but hidden.

📋 Required Actions

To proceed with merging, please address the following:

• Fix test/playwright/e2e/NeuralLinkChildappConnect.spec.mjs so the moved colors-childapp proof passes at exact head. The current visible wait is too strong for an intentionally empty viewport; either wait for the stable #colors-widget-viewport to be attached before the Neural Link readback, or make the fixture surface visibly mounted if visibility is genuinely part of the capability being asserted.
• Rerun and report the exact focused e2e evidence after the fix, including NeuralLinkChildappConnect and NeuralLinkWindowOps, then update the PR body if the current evidence text changes.

📊 Evaluation Metrics

• [ARCH_ALIGNMENT]: 88 - Correct deletion direction and correct preservation of the bare popup host; capped by the red replacement proof.
• [CONTENT_COMPLETENESS]: 78 - Strong PR narrative and amended boundary explanation, but the evidence claim overstates current exact-head behavior.
• [EXECUTION_QUALITY]: 58 - Diff hygiene and stale-reference cleanup are good; a changed e2e test fails locally.
• [PRODUCTIVITY]: 86 - Removing the wrong-shape spine is high-ROI damage control despite the narrow test blocker.
• [IMPACT]: 82 - Deletes a large attractor for the superseded product direction while protecting multi-window host behavior.
• [COMPLEXITY]: 55 - Large deletion but narrow surviving behavior: host shell plus two Neural Link proofs.
• [EFFORT_PROFILE]: Maintenance - Cleanup/retraction PR with a focused coverage repair required.

This should be a quick repair, not a new planning loop. The merge gate should stay closed until the replacement childapp-connect proof is actually green at the PR head.