[issue-3312][slice-3/4] Decompose gateway/gateway.py (10,648... - #3503
Conversation
…eline) Step-0 bisectable baseline for the #3312 gateway.py decomposition (slice-3 of #3312's continuation, the slice-18-equivalent Flask @app.route seam). Pure git mv with no extraction: the sub-package __init__.py is the pre-move file with only the mechanical package-form fixups, so the gateway.gateway module path and every route/handler are unchanged. Cluster extraction lands in the next commit. Package-form fixups (no behaviour change): - Relative sibling imports .X -> ..X (the module is one level deeper now); each already has an absolute `from X import` except-fallback for flat/container mode. - _shared_path / _config_path gain one .parent to keep the same targets. - gateway/tests/conftest.py spec-loads the package (submodule_search_locations), registering it as both `gateway` and `gateway.gateway` so patch("gateway.gateway.*") and the flat `import gateway` both resolve — mirrors the git_client/worktree_manager slice-11/12 loaders. Container packaging + Flask launch (R3, same slice): - gateway/Dockerfile: explicit `COPY gateway/gateway/ ./gateway/` (the non-recursive `COPY gateway/*.py ./` no longer matches the package dir). - gateway/gateway/__main__.py + entrypoint.sh `python3 -m gateway` (replacing `python3 gateway.py`) preserve the Flask launch on port 9848. NOTE: the in-image `python -c 'import gateway'` / serve-on-9848 smoke check could not run in this sandbox (no docker); COPY/launch follow the established git_client/worktree_manager pattern. Part of #3312. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Second commit of slice-3 (the #3312 slice-18-equivalent Flask @app.route seam): decompose the pure-move baseline into a barrel + 14 responsibility-grouped submodules, all under the 1500-line / 100 KB caps (largest _git_ops.py, 1357 lines / 57.9 KB; barrel 1342 lines). Pure refactor — handler/helper bodies are AST-identical to the pre-split file. Routes-handling convention: every @app.route decorator stays on a thin wrapper in the barrel (__init__.py); the wrapper delegates to the implementation in the _<cluster> submodule. The barrel does explicit per-symbol re-exports and declares __all__, so external importers and unittest.mock.patch targets (patch("gateway.gateway.X") / patch.object(gateway, "X")) resolve unchanged. Seam preservation: - _b() accessor: submodules resolve *patched* seam getters/validators on the barrel at call time, so patch("gateway.gateway.<name>") stays effective; non-patched cross-submodule helpers use direct typed imports. - _BarrelLogger proxy: submodule `logger` forwards to the barrel logger so tests patching gateway.logger observe submodule log calls. - Module-singleton seams (gateway.subprocess.run / gateway.time.sleep / gateway.open) stay barrel attributes; patching the shared module is honoured process-wide. Submodules: _helpers (responses/audit/connectivity), _health, _git_ops (push/fetch), _git_execute, _gh_ops (PR lifecycle), _gh_execute, _jira (reads), _jira_writes, _confluence, _worktree, _sessions, _proxy (Anthropic /v1/messages), _server (main + health server). Drops gateway.py's file-size-allowlist entry (the barrel + submodules are all under cap); the files: map now holds only orchestrator/routes/pipelines.py (slice-4's target). Seam-table data for gateway/CLAUDE.md handed to the documenter via .egg-state/agent-outputs/coder/slice-3-seam-table-for-documenter.md. Verification: full gateway/tests suite passes (3342 passed; the 7 failures + 6 errors are all pre-existing sandbox-env failures — git-init-blocked git_client/worktree_manager tests and squid-403 health-server tests, identical to the pre-split baseline). ruff check + format clean. Part of #3312. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
) Add the gateway/gateway/ subsection to gateway/CLAUDE.md's Decomposition seams table (task-3-5 doc portion): barrel + 14 submodules, routes convention (@app.route decorators stay on thin barrel wrappers delegating to _<cluster> impls), barrel re-export/__all__ + _b()/_BarrelLogger/module-singleton seam mechanism, and the R3 Dockerfile COPY + python3 -m gateway launch note. Stacks on the coder's decomposition at 3a85636. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![james-in-a-box[bot]](https://avatars.githubusercontent.com/u/8340608?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contract Verification — PR #3503 (issue-3312-v2, slice-3/4)
Verified the gateway.py decomposition against the 7 slice-3 tasks and their acceptance criteria. Reviewed the working tree at head 711b0de and exercised the imports directly. Result: structurally sound, all code-level criteria met — one AC (task-3-6 container smoke check) could not be executed in-sandbox and needs CI/human validation before merge.
Task-by-task
task-3-1 — External-importer audit / re-export set ✅
Barrel re-exports a full public surface (__all__ = 187 entries). Spot-checked externally-referenced symbols resolve as barrel attributes: get_anthropic_client, GitHubClient, WorktreeManager, audit_log, logger, subprocess, time, make_response. Seam-table artifact produced for the documenter.
task-3-2 — Pure-move baseline; app + routes register identically ✅
import gateway.gateway yields a Flask app with all routes registered (61 url_map rules incl. /api/v1/health, /api/v1/git/push). Old gateway/gateway.py is removed. (The intermediate move-only commit tree isn't inspectable from this single-commit worktree, but the final state is consistent with the PR's 4-commit structure.)
task-3-3 — Routes-handling convention ✅
All 50 real @app.route decorators live in __init__.py; the 13 matches outside the barrel are module-docstring references, not decorators. Wrappers are thin and delegate to _<cluster> impls (e.g. git_push() → _git_ops.git_push()). patch("gateway.gateway.<symbol>") targets resolve; conftest registers the package under both gateway and gateway.gateway so flat + dotted paths both resolve.
task-3-4 — Cluster sizing under caps ✅
Every submodule is under both caps. Largest: _git_ops.py 1357 lines / 57.9 KB; barrel __init__.py 1342 lines / 44.7 KB. No submodule > 1500 lines or 100 KB; no nested further-split needed; zero new allowlist entries.
task-3-5 — Drop allowlist entry + CLAUDE.md seam doc ✅
gateway.py removed from scripts/file-size-allowlist.yaml; only orchestrator/routes/pipelines.py (slice-4's target) remains. gateway/CLAUDE.md gains a concrete gateway/gateway/ seam section documenting the barrel + 14 submodules, the routes convention, and the _b()/_BarrelLogger/module-singleton seam mechanism.
task-3-6 — Dockerfile COPY + Flask launch
gateway/Dockerfile:80addsCOPY gateway/gateway/ ./gateway/✅- Launch converted to
python3 -m gateway --host 0.0.0.0 --port 9848inentrypoint.sh(both gosu and fallback paths), via__main__.py→ barrelmain()✅ appandmainare exported through the barrel (mainin__all__;appis a module attribute, importable asfrom gateway.gateway import app✅ — see nit below)config_validator.pydoes not importgateway, so no consumer update was needed ✅- Gap: the AC requires "built image passes a start/serve smoke check". The commit message states the in-image
import gateway/ serve-on-9848 check could not run (no docker in sandbox). The COPY + launch changes mirror the already-landedgit_client/andworktree_manager/patterns and both import paths verify locally, so risk is low — but the container build/serve check is not evidenced. Flagging for CI or a human reviewer to confirm the image builds and serves on 9848 before merge.
task-3-7 — Green + no behavior change ⏳ deferred to CI
Per review conventions I did not run make test-all. Coder reports 3342 passed with 7 failures + 6 errors that are pre-existing sandbox-env failures (git-init/squid-403), and claims AST-identical bodies. Import smoke test + wrapper→impl delegation support no-behavior-change; CI is authoritative for the green gate.
Minor nit (non-blocking)
gateway/CLAUDE.md states the barrel __all__ keeps app in the public API, but app is not literally in the __all__ list (only main is). Functionally harmless — from gateway.gateway import app resolves regardless (__all__ only governs import *, which nothing here uses) — but the doc slightly overstates it.
Contract bookkeeping note
The contract (issue-3312-v2) still shows slice-3 and all 7 tasks as pending with phase refine and no linked commits, and has no top-level ac-N acceptance criteria (empty array). So there were no ac-N entries to mark via verify-criterion, and the orchestrator was unreachable. Verification above is against each task's acceptance_criteria string. The contract task/phase status was not advanced as the work landed — worth reconciling.
Verdict
No correctness defects found; the decomposition is a clean, cap-compliant refactor that matches the contract. The only outstanding item is the unexecuted container start/serve smoke check (task-3-6) — a sandbox limitation, not a code defect — which CI/human review should confirm before merge. Posting as a comment (bot self-authored PR).
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Feedback addressed — PR #3503Thanks for the contract verification. Per-item disposition: 1. Minor nit — 2. task-3-6 — container start/serve smoke check could not run in-sandbox (no docker); flagged for CI/human confirmation. — 3. Contract bookkeeping — — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Contract Verification (re-review) — PASS
Re-verified slice-3 (gateway/gateway.py decomposition) at 64fa3077. The delta since my last review (711b0de) is a single one-line change: "app" added to __all__ in gateway/gateway/__init__.py, in correct alphabetical position (after _worktree_prune_lock, before artifact_bp).
Delta assessment
- Correct and minimal.
appis defined in the barrel (app = Flask(__name__),__init__.py:513) and is now an explicit public export. This matches the documented public API ingateway/CLAUDE.md:80("keeping the full public API (app,main, …) as the stable surface") — the code now agrees with the doc. - Directly satisfies task-3-6 AC ("
appis exported through the barrel"). - No regression risk. Purely additive to
__all__. Import smoke test: package imports cleanly,g.appis aFlaskinstance,g.mainis callable, and every one of the ~200 names in__all__resolves as an attribute (0 dangling) — the newappentry does not dangle.
Previously-verified criteria re-confirmed (no regressions)
- task-3-3 (decorators stay in barrel): all 49 real
@app.routedecorators are in__init__.py; zero in the cluster submodules (the 13 submodule matches are identical docstring lines). ✓ - task-3-4 (sizing): every submodule under both caps — largest is
_git_ops.pyat 1,357 lines / 57.9 KB (< 1,500 lines / 100 KB). ✓ - task-3-5 (allowlist + seam doc):
gateway.pyentry absent fromscripts/file-size-allowlist.yaml;gateway.pyfile removed;gateway/CLAUDE.mdcarries the concretegateway/gateway/submodule seam table. ✓ - task-3-6 (packaging + launch):
gateway/Dockerfile:80addsCOPY gateway/gateway/ ./gateway/;entrypoint.shlaunchespython3 -m gateway --host 0.0.0.0 --port 9848→__main__.py→main()(in_server.py) →app.run(port=9848). Port 9848 unchanged. ✓
Notes
- No top-level
ac-Nacceptance criteria exist in this contract (criteria are per-task strings within the slice), so there are noverify-criterionids to mark. - The orchestrator was unreachable during this review, so contract state was read from the frozen worktree snapshot (
.egg-state/contracts/issue-3312-v2.json); code was verified directly against the tree. - I did not run
make test-all(per review policy — left to CI).
Verdict: No contract violations. The delta is a correct, self-contained public-API fix. Posting as a comment per the self-authored-PR convention.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Feedback addressed — PR #3503Thanks for the re-review. The latest contract verification at 1. Re-review delta ( 2. Notes — no top-level No code changes were required for this round — the re-review is a confirmation of the already-pushed fix. PR HEAD remains — Authored by egg |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Contract re-verification — incremental (delta since 64fa307)
Verdict: Approve (comment). The delta since my last review is a single merge commit (26400837) that merges main into the slice-3 branch and resolves the scripts/file-size-allowlist.yaml conflict. It preserves all previously verified work and introduces no contract violations. (PR is already merged; this is a confirming record.)
Delta reviewed
64fa3077..26400837 — the merge "Merge main into slice-3; drop gateway.py allowlist entry, keep concurrent_executor.py". The only PR-authored change in the merge is the allowlist conflict resolution; the remaining 87 commits are main history pulled in by the merge.
Conflict resolution is correct
scripts/file-size-allowlist.yaml files: now contains:
orchestrator/routes/pipelines.py— untouched (slice-4 scope, not this PR)orchestrator/concurrent_executor.py— kept (main's addition) ✓gateway/gateway.py— removed (this slice's deliverable) ✓
This is exactly the resolution the commit message describes: take main's concurrent_executor.py entry, keep this branch's removal of the gateway.py entry. No cross-contamination.
Previously verified work still holds
gateway/gateway.pydecomposed intogateway/gateway/(barrel__init__.py+ 14 sub-modules). All modules are within caps (max 1357 lines / 57.9 KB vshard_lines: 1500/hard_bytes: 100000), so dropping the allowlist entry is legitimate — no file is over cap.- Barrel re-exports intact:
from ._confluence/._gh_ops/._git_ops/...with# noqa: E402,F401;app = Flask(__name__)andget_anthropic_clientexposed."app"is present in__all__(the 64fa307 fix survived the merge). - Dockerfile COPY parity (R3):
COPY gateway/gateway/ ./gateway/(Dockerfile:80) with the explanatory comment; entrypoint launches viapython3 -m gateway(entrypoint.sh:343/352), matchinggateway/__main__.py. - No merge-conflict markers left in any PR-authored file.
Acceptance criteria
Contract top-level acceptance_criteria is empty ([]), so there are no ac-N entries to mark via verify-criterion. All four slices in issue-3312-v2.json (models.py, event_loop.py, gateway.py, pipelines.py) are status: complete; this PR covers the gateway slice, whose per-task criteria (external-importer re-export audit, byte-cap compliance, Dockerfile parity) are met as verified above.
No blocking issues.
— Authored by egg
|
egg contract-verification completed. View run logs 6 previous review(s) hidden. |
1 participant
Decompose
gateway/gateway.py(10,648 lines / 419 KB) into a sub-package; drop its allowlist entry; seam coverage in gateway/CLAUDE.md. Depends on the previous slice (single linear chain).Base PR: #3489
What's in this PR
Commits (4):
This slice
Decompose gateway/gateway.py (10,648 lines, STRUCTURAL OUTLIER, OVER BYTE CAP): Flask @app.route seam (#3312 slice-18 equivalent)
Files affected:
gateway/gateway.pygateway/gateway/__init__.pygateway/gateway/scripts/file-size-allowlist.yamlgateway/CLAUDE.mdgateway/Dockerfilegateway/entrypoint.shgateway/tests/tests/Tasks (7) + acceptance criteria
COPY gateway/*.py ./. Converting gateway.py -> gateway/gateway/init.py needs TWO same-slice fixes handled together: (a) addCOPY gateway/gateway/ ./gateway/to gateway/Dockerfile (mirroring the git_client/worktree_manager lines) so the new package dir ships; (b) preserve the Flask launch: gateway.py definesapp = Flask(__name__)(L486) andif __name__=='__main__': app.run(...)(L10639/L10647), started on port 9848 via gateway/entrypoint.sh; keep the sameappobject exported through the barrel and update the launch invocation / anyimport gatewayconsumer (incl. config_validator.py) so the server still starts. BUILD the gateway image and smoke-check the container starts and serves on 9848.appis exported through the barrel and the launch path (entrypoint.sh) starts the Flask server on 9848 unchanged; built image passes a start/serve smoke check.Stack
issue-3312-v2egg/issue-3312-v2/slice-2