open-swe/docs/upstream-sync/domain-reorg/reorg-build-plan.md
Adam Moussa 373d888426
docs: land reorg in ledger, fix path refs, ship plan artifacts (C7)
C7 of the domain-reorg adoption (build plan docs/upstream-sync/domain-reorg/
reorg-build-plan.md, step C7):

- CLAUDE.md / AGENTS.md: retarget architecture path references to the new
  layout — graph entrypoints via agent.graphs.* shims, the agent/api/ +
  agent/webhooks/*_routes.py FastAPI split (webapp.py now a shim),
  agent/review/ package, tests/<domain>/ test paths, and the new-graph/
  test conventions. README.md already pointed at docs/ (C1) — no change.
- triage.jsonl: flip 8356eb34 (#1726) to landed on branch
  refactor/domain-reorg-adoption; add re-triage notes to the 8 unblocked
  rows (#1732/#1761/#1744/#1748/#1742 clean, #1736/#1758/#1760 near-clean).
  triage.md regenerated via make triage-render.
- Ship the plan, scoping report, move-map artifacts, and the
  domain-reorg-adoption workflow so the exercise is reproducible.

Memory + Confluence handled out-of-band (not in this commit): project
memory updated with the new module layout; Confluence check found no IT
page documents the repo module map — no Confluence change required.
2026-07-17 15:01:45 -04:00

66 lines
12 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Build plan — adopt upstream domain reorg `8356eb34` (#1726) into the Sea Haven open-swe fork
Date: 2026-07-17. Base: `dev` (clean, `a518a129`). Scoping report: `reorg-scoping-report.md` (same directory: `docs/upstream-sync/domain-reorg/`).
## Goal & strategy
Adopt upstream's 298-file domain reorg as a **file-move exercise applied to the fork's tree**: fork file contents, upstream layout. Never take upstream file contents except the 21 verified thin structural "A" shims (`agent/graphs/*`, `agent/runtime/*`, `agent/api/*`, `agent/webhooks/{common,*_routes}.py` skeletons, package `__init__.py`s). Rationale: the fork has settled permanent divergences (sync sandbox lifecycle + 4-case `__creating__` sentinel, workflow-token security model, bun toolchain, Atlassian integration, plan-mode #130, TID-COLLIDE-01 guard) — adoption is layout convergence, not content convergence.
Payoff: kills the per-pick path-remap tax on all future upstream cherry-picks; 8 of 19 deferred ledger rows become clean/near-clean, including priority security pick #1736 (written against `agent/webhooks/common.py`, which only exists after this exercise).
## Approved decisions (Adam, 2026-07-17)
1. **FastAPI/webapp split: APPROVED.** Fork's 2,590-line `agent/webapp.py` splits into `agent/webhooks/common.py` (shared verify/dispatch helpers), `agent/api/{app,health}.py`, and per-source `{github,linear,slack,jira,confluence}_routes.py`; `webapp.py` remains a compatibility shim. `/sh-security-review` required on that commit before merge.
2. **Atlassian placement (re-confirmed by Adam 2026-07-17):** fork-only `jira_routes.py` + `confluence_routes.py` mirroring upstream's per-source pattern; **Atlassian Connect lifecycle/descriptor routes (`/connect/*`) fold into `confluence_routes.py`** (Connect is the Confluence trigger; Jira uses Automation webhooks). JWT/qsh machinery stays in `agent/utils/atlassian_connect.py` (unmoved). Split out a `connect_routes.py` later only if Jira migrates to Connect.
3. `langgraph.json` graph entrypoints retarget to `agent.graphs.*` shims; add fork-only `agent/graphs/ci_monitor.py` shim for symmetry. `http.app` stays `agent.webapp:app`.
4. Reviewer modules → `agent/review/` package (9 pure moves; fork reviewer content rides along).
5. CI-autofix cluster (upstream-deleted): keep-baseline; wire its webhook triggers through the split `github.py`/`common.py`; tests → `tests/github/`.
6. UI `ported/` → `features/agents/experiments/` + single tsconfig/eslint exclude glob; bun stays; delete stray `ui/pnpm-workspace.yaml`.
7. Sequencing: reorg lands **before** #1736.
## Hard rules (from fork-maintenance playbook + scoping hazards)
- **Content-source discipline:** upstream content only for the 21 "A" shims. `agent/webhooks/{github,linear,slack}.py`, `tests/conftest.py`, `tests/e2e/harness.py` exist on both sides with different content — always fork content + replicated import rewires; never `git checkout upstream` on them.
- Handlers keep **module-attribute access style** (`common.X`, `service.X`) exactly as upstream did, so the 240 `monkeypatch.setattr(webapp, ...)` test sites retarget cleanly and no re-import rebind hazard is introduced.
- Every moved test keeps its impl-side pairing (impls mostly don't move; 13 fork-only tests get the placements from scoping §2c: Atlassian webhook tests → `tests/webhooks/`, `test_atlassian_connect.py` → `tests/auth/`, client-util tests → `tests/tools/`, `test_repo_binding_isolation.py` → `tests/sandbox/`, `test_auth_error_leak.py` → `tests/auth/`, `test_app_bot_identity.py`/autofix tests → `tests/github/`).
- Verify wheel packaging ships `agent/resources/default_prompt.md` (hatchling `packages=["agent"]` should carry it — build and inspect the wheel in C1; add explicit include only if missing).
- No secrets/config placement changes anywhere in this exercise (no new env vars, no Secrets Manager/SSM changes).
- CI ladder per commit: `ruff check` + `ruff format --check` → `pytest --co -q` → full unit → E2E (Playwright + real LangGraph dev server) where the commit touches wiring; `bunx tsc --noEmit` + bun build for UI commits. `make dev` boot-check for C3 (all graphs + FastAPI app load).
## Jira linkage
No Jira issue tracks this work; per handbook `git-workflow.md`, the branch omits the issue key and uses a plain description. (If Adam cuts a ticket, rename branch to `refactor/<KEY>-domain-reorg-adoption` before the PR opens.)
## Commit sequence (branch `refactor/domain-reorg-adoption` off dev; one PR; **merge-commit merge ONLY** — squash and rebase are explicitly forbidden for this PR to preserve per-cascade bisectability; dev ruleset allows merge commits)
- **C1 — docs/resources/assets** (~1h): `git mv` `INSTALLATION.md`/`CUSTOMIZATION.md` → `docs/`, `static/` → `assets/` (fix README refs), `default_prompt.md` → `agent/resources/` + apply the `importlib.resources` loader hunk to fork's `prompt.py` + `__init__.py`. Gate: ruff + unit + **wheel build inspection**.
- **C2 — `agent/review/` package** (~half day): mv 9 reviewer/style modules per map, rewrite 38 importer files, `agent/review/__init__.py` with fork's export surface. Gate: ruff + `pytest --co` + reviewer unit suite.
- **C3 — graphs/runtime/providers shims + `langgraph.json` + CI hardening** (~2h): adopt the 21 upstream A-shims (verify each delegates to fork modules), add `agent/graphs/ci_monitor.py`, retarget `langgraph.json`; **pin `bun-version` in `.github/workflows/ci.yml`** (removes the setup-bun latest-resolution flake documented in project memory). Gate: `pytest --co` + `make dev` boots all graphs (incl. `ci_monitor`) + FastAPI app.
- **C4 — FastAPI split** (~1–1.5 days, critical): split fork `webapp.py` per approved decisions 1–2; sed handler modules `webapp.` → `common.`; retarget the 240 monkeypatch sites, `tests/conftest.py`, `tests/e2e/harness.py`; keep `webapp.py` shim. Gate: full unit + **full E2E** (the only layer catching wiring drift) + **residual-importer sweep**: `git grep -l 'agent\.webapp\|from agent import webapp'` across the whole tree must return only the shim and intentional compat references, so the shim cannot silently mask missed rewires. **`/sh-security-review` on this commit's diff. Failure path: findings are addressed as NEW commits on top of the branch (e.g. "C4a: address security-review findings") — no history rewrite or force-push once any review (security or PR) has started, preserving the review/CI trail; the PR must not merge until the review passes — no exceptions.**
- **HARD GATE:** C5 must not begin until C4 has full unit + E2E green **and** `/sh-security-review` passed. (C1–C3 may proceed independently before C4.)
- **C5 — `tests/<domain>/` moves** (~half day): mechanical map moves + remaining R<100 monkeypatch retargets + the 13 fork-only placements. Gate: `pytest --co` + full unit.
- **C6 — UI `features/` moves** (~half day–1 day): `git mv` per map incl. `ported/` → `experiments|chat`, rewrite `@/` imports (54 importers + moved files), exclude-glob swap, AgentPromptBar shim retarget, delete `ui/pnpm-workspace.yaml`. Gate: `bunx tsc --noEmit` + bun build + Playwright E2E.
- **C7 — docs + ledger + memory** (~2h): update fork `CLAUDE.md`/`README.md`/`AGENTS.md` path references (CLAUDE.md architecture sections name `agent/webapp.py` and old test paths); flip `8356eb34` → Landed in `triage.jsonl` + re-triage notes on the 8 unblocked rows + `make triage-render`. **Documentation/memory obligations (owned by the C7 executor, i.e. Claude in this conversation):** (a) update the `open-swe-upstream-triage-status` and project memory entries **including a summary of the new module layout** (`agent/{graphs,runtime,api,review,resources,webhooks/*_routes}`, `tests/<domain>/`, `ui/src/features/`); (b) perform the Confluence check — search IT-space pages for any repo module-map documentation; update it if found, and **record the outcome either way ("updated page <id>" or "no Confluence change required — no IT page documents the module map") in the PR description and in project memory**. The AWS Architecture Map is unaffected (no AWS resource changes). **Fallback per standing policy: if Confluence is inaccessible, state so explicitly in the PR description and project memory — the documentation update is then outstanding, not silently skipped.** C7's doc/ledger changes ship inside the PR, so they are covered by the PR review; memory-file updates are summarized in the PR description for the same visibility.
## Merge & deployment
- PR from `refactor/domain-reorg-adoption` → `dev`; CI green (all layers) → `@openswe` review (or Adam-delegated review) → **merge commit only** (no squash/rebase). PR review scope must explicitly name the C2 reviewer-module moves (review-automation surface) and the C4 auth-surface split so the reviewer examines both.
- `/sh-security-review` must have run on C4 with confirmed critical/highs resolved (or machine-suppressed with justification) before merge — see C4 failure path. GPT-4.1 cross-family review: not strictly triggered (no IAM/policy or Lambda-signature change — `publish_review`-style tool contracts untouched; `langgraph.json` strings are deployment manifest, not IAM), but run as **opt-in on the C4 diff** given webhook-verification code movement.
- **Post-deploy verification checklist (explicit, before declaring done):** LangGraph deployment redeploys with the retargeted `langgraph.json`; all graphs (agent, reviewer, analyzer, chat, scheduler, ci_monitor) registered; `/health` OK; `GET /connect/atlassian-connect.json` serves the descriptor; one webhook round-trip per source (GitHub PR comment, Slack mention, Linear, Jira Automation test-fire, Confluence comment) where feasible. Note: external registrations (GitHub App webhook URL, Slack event URL, Jira Automation rule, Connect descriptor URL) are verified UNCHANGED by this exercise — the HTTP surface and all routes keep their paths — so no external system updates are required; the checklist confirms this empirically rather than assuming it.
- **Rollback:** revert the merge commit restores the pre-reorg layout wholesale; because the HTTP surface and external registrations are unchanged, a rollback requires NO external-system reverts — re-verify with the same checklist after any rollback. C1–C3 independently revertible pre-merge; C5/C6 path-only.
- **Post-merge cleanup (git-workflow convention):** delete `refactor/domain-reorg-adoption` on origin after merge; delete local copies and prune worktrees.
- **Verification records:** post-deploy checklist results are recorded in the PR description (or a comment) as the traceability artifact — **responsibility: the C7 executor (Claude in this conversation)**. After any rollback, normal operation does not resume until the same checklist fully passes.
- **Confluence-outstanding follow-up:** if Confluence is inaccessible at merge time, the outstanding update is recorded in project memory with an explicit "outstanding as of <date>" marker so it surfaces at the next session on this project — it does not defer indefinitely.
- **Severity clarification for the C4 failure path:** fixes are stacked as new commits regardless of severity, full stop. If the security review reveals a *fundamental design flaw* in the split (not a fixable finding), the response is not a history rewrite — the PR is closed, the plan returns to the drafting stage, and a revised plan goes back through `/sh-plan-review` on a fresh branch.
- **Shim lifetime:** `webapp.py` (and the `agent/graphs/*` shims) mirror upstream's own permanent compatibility shims — they are kept for as long as upstream keeps theirs, since layout parity is the goal; revisit only if upstream removes them.
- **bun-version pin:** permanent policy (version bumped deliberately like any dependency), not a temporary workaround — resolves the setup-bun latest-resolution flake class permanently.
## Post-merge follow-ups (separate work, not this PR)
1. Pick #1736 (GitHub token binding) — near-clean post-split; still gated by its ledger row: GPT-4.1 cross-family review + `/sh-security-review`.
2. Re-triage the 5 now-clean picks (#1732, #1761, #1744, #1748, #1742) — cheap batch.
3. Update `sh-openswe` deployment notes/memory if the redeploy surfaces anything.
## Rollback story
Single revert of the merge commit restores pre-reorg layout wholesale. Before merge: each cascade commit is independently droppable except C4→C5 ordering (C5 assumes post-split monkeypatch targets). Working artifacts for the build agents: `docs/upstream-sync/domain-reorg/{movemap-m50.txt, cross.json, forkonly.json}`.