Split authz.py into package

Task 7604 · Done · Coder · created · updated

## Requirement (verbatim, epic 7555) > Break large modules into smaller and more structured ones. E.g. "Very large modules: routes/[tasks.py](http://tasks.py) has 12,867 lines and [models.py](http://models.py) has 8,770." Settled upstream (PRD = task 7553, Architecture = task 7554): split the 26 files over 3,000 lines frozen on 2026-10-18 — one domain per module, one PR per module, every caller rewritten with no shim, external contracts and DB schema unchanged, no size gate. This child is ONE of 21 independent PRs; no child depends on another. ## This child: `apps/api/app/authz.py` (3,072 lines) → package `apps/api/app/authz/` (Architecture §5.10). No paired test file over 3k. Modules: - `catalog.py`: `FEATURES`, `PERMISSION_CATALOG`, `OWNER_KEYS`, `Access`, `Tenancy`. - `declarations.py`: `Declaration`, `declare`, `feature`, `superadmin_only`, `org_admin`, `infra`, `always`, `declaration_of` (the `__authz__` attribute keeps working; no registry is keyed by module). - `principal.py`: `Principal`, `principal_for_user`, `current_principal`, `Caller`, `_PRINCIPAL_CACHE_ATTR`. - `visibility.py`: SQL `visible_*_clause`, `ObjectSpec`/`OBJECT_SPECS`, `scoped_*_select` (~860 lines). - `object_access.py`: `can_access`, `require_object_access`, `validated_ownership`, `stamp_owner_on_create`, `_ACTION_OBJECT_ACCESS`. - `enforce.py`: `HintedDenial`, `_enforce`, `enforce_in_process`, `_tenancy_disagreement`. - `guard.py`: `ORG_SCOPE_HEADER`, `authz_guard`. Patch seam (C2): `monkeypatch.setattr(authz, "current_principal")` appears 15× in 7 files — its callers in `enforce.py` and `guard.py` use `principal.current_principal`, and tests patch `app.authz.principal`. `can_access` is patched on `routes.tasks` today (`test_tasks_routes.py:1345`): wherever its consumers already reach it by module attribute, keep them on `object_access.can_access`. Repoint (C5): `test_authz_policy.py` — the `app/authz.py` path skip now covers the `app/authz/` dir (:2379–2383, :2550); `test_org_filter_sites.py:114–265` (`authz.py::<fn>` keys → new file). Ownership: `.github/CODEOWNERS:29` `/apps/api/app/authz.py` → `/apps/api/app/authz/`. AD2 migration (sanctioned narrow exception to N8): `apps/api/migrations/versions/202610130700_baseline.py:1413` function-local `from app.authz import PERMISSION_CATALOG` → `from app.authz.catalog import PERMISSION_CATALOG`; nothing else in that file changes. Re-verify the line at rebase. Living docs: 9 doc files point at `authz.py` (incl. CLAUDE.md router rows). AD10 security surface (authz): merge permission is granted by the project instructions — do not ask. Still run the code-review lane's SECURITY profile (every guard, declaration and visibility clause moved verbatim; no permission or tenancy check weakened, reordered or dropped) and record its verdict in the PR body. ## Shared rules (Architecture §3, binding) - C1 Package shape: `X/__init__.py` is a docstring module map only — no names, no `__getattr__`, no `*`. - C2 Patch seams: a moved name that tests patch is reached by other new modules as `<module>.<name>` via `from app.authz import <module>`; tests patch it on the defining module; drop `raising=False` once the new home is confirmed. - C3 Codemod import forms: parenthesised `from app.X import a, b`; `from app import authz` + `authz.attr`; `import app.X as m`; string targets `"app.X.name"`/`"app.X:name"`; `sys.modules["app.X"]`; function-local imports stay function-local. - AD6 Codemod: a transient rewrite script (stdlib `ast` or `sg`; no libcst) kept in `$JAAH_REPORT_DIR`, never committed; re-run after every `git rebase origin/develop`. Scope: `apps/ tools/ infra/ packages/` plus the AD2 line. - C5 Source-reading and path-keyed tests/registries are repointed, never deleted or skipped. Discovery: `git grep -nF 'app/authz.py' -- tools/ apps/ infra/ packages/ '*.toml' '*.json' '*.yml' '*.sh' .github/`; re-key every hit. - C6 `logging.getLogger(__name__)`; update logger-name assertions; no infra metric filter keyed on the old dotted path. - C7 Every living-doc pointer to the moved path points at the package dir or owning submodule; a deleted path never appears in backticks. - Every new module opens with a one-line docstring naming the one domain it owns; no `utils`/`common`/`helpers` grab-bag. - AD9 Delivery: merge lane is ON — do NOT arm auto-merge. Rebase on fresh `develop` and re-run the codemod immediately before each `pull_request_enqueue`; enqueue after every push. Watch Jaah Deploy: on failure fix and enqueue again; cancelled/superseded → leave it. - Run `./dc check full` only on the committed, rebased tree, never in parallel with `./dc test` on the same slot. - Small internal cleanups allowed only where no caller outside the module can observe them. Credentials & reach: - AC5, AC6, AC7, O4, O-patch, O-catalog, O-mappers, O-cycle, O7 build | None beyond the worktree; the slot's local `./dc` stack (project p1-6) | verified on node h2: `./dc up` exit 0; `./dc run api alembic check` → "No new upgrade operations detected."; the in-process `app.openapi()` returned 726 paths and 1,037 schemas - AC9 / AD9: PR view, enqueue, Jaah Deploy watch | `jaah-forge`, pre-authenticated for the task, plus the `pull_request_enqueue` control tool; GitHub `jaah-ai/jaah` | verified: `jaah-forge api get repos/jaah-ai/jaah/actions/workflows` from node h2 listed the "Jaah Deploy" workflow - AC10 dev UI check, O7 node-healthy check | `jaah/debug/main/operator-login` (roster; covers app.jaah.ai and app.dev.jaah.ai); env `https://app.dev.jaah.ai` | verified: `credential_get` succeeded (value not recorded); HTTPS GET of app.dev.jaah.ai from node h2 returned 200. `dev.jaah.ai` (internal ALB) timed out from h2, so it is not used. `gmind/skynet/dev` is on the roster but its base_url is an old host, so it is not used. - AC10 "a fleet runner claims a ready Task"; O7 "dev node healthy" | Same operator login; the console Nodes and Tasks pages on app.dev.jaah.ai | verified: signed in to app.dev.jaah.ai as the operator login in a browser from h2; the Nodes page listed 3 nodes, all `online` (including ts2-node6), which are the runners a deploy updates Out of scope: Do NOT touch `packages/agent-sdk/examples/python/**` (N1). Do NOT split any other file — the other frozen files each have their own child (N7), and files ≤3,000 lines today are follow-ups (N2). Do NOT add a line-count ceiling, lint rule or gate (N3). Do NOT keep `app.authz` names importable through a shim, facade, `__getattr__` redirect, star re-export or alias module (N4). Do NOT change any permission, feature key, declaration, HTTP paths/methods/schemas, DB schema (no new Alembic revision), node↔CP wire format, env-var names, or user-visible UI (N5). No new features, dependency bumps or behavioural fixes riding along (N6). Do NOT rewrite git history; in `apps/api/migrations/versions/` touch ONLY the AD2 baseline import line (N8 with the AD2 exception). Done means: - AC1 No §4 file remains over 3,000 lines, and no module created by the split is over 3,000 lines. — Run `git ls-files | grep -E '\.(py|ts|tsx|js|sh)$' | grep -v -E 'node_modules|alembic/versions|migrations/|packages/agent-sdk/examples/' | xargs wc -l | awk '$1>3000 && $2!="total"' | sort -rn` on `develop` after the PR; observe `authz.py` absent and no new path under the split directory listed. - AC2 Each new module owns one domain and declares it (R2). Fails if a module is a grab-bag (`utils2`, `misc`, `part2`, `helpers` mixing domains) or its docstring names two unrelated domains. — `for f in <new files>; do head -3 "$f"; done` → every file opens with a one-line ownership docstring/comment; reviewer (code-review lane) confirms each module's contents match its stated domain and its name. Record the module→domain table in the PR body. - AC3 Old path removed, no shim (R3). — Architecture's restated AST oracle (stdlib-`ast` script in `$JAAH_REPORT_DIR`) over `apps/ tools/ infra/ packages/` (migrations included) for P=`app.authz` fails on: (a) `from P import N` where N is not a submodule file; (b) `from app import authz [as m]` / `import app.authz [as m]` with any `m.<attr>` naming a non-submodule; (c) `from P.<sub> import N` where N is not defined in that submodule; (d) a string target `"P.<Name>"`/`"P:<name>"` whose first component after P is not a submodule; (e) the package `__init__` AST check `python -c "import ast,sys;t=ast.parse(open(sys.argv[1]).read());print([n for n in t.body if not isinstance(n,(ast.Expr,ast.Import,ast.ImportFrom)) or isinstance(n,ast.ImportFrom) and n.names[0].name=='*'])" <init>` printing anything but `[]`. Cite the zero-finding output in the PR body. - AC4 Routes unchanged (A-2). — O4: on the base sha and after, `./dc run api python -c "import json; from app.main import app; print(json.dumps(app.openapi(), sort_keys=True))" | tail -1 > openapi.json`; `diff <(jq -S .paths before.json) <(jq -S .paths after.json)` and the same for `.components.schemas` are both empty. - AC5 DB schema unchanged (A-2). — `alembic check` in the `./dc` API container prints "No new upgrade operations detected"; the PR adds no file under `apps/api/migrations/versions/`; `git diff origin/develop -- apps/api/migrations/versions/` shows only the AD2 baseline line. - O-cycle: in the `./dc run api` container (`cd /app`, no `-I`), `python -c "import app.authz.catalog"` and `python -c "import app.authz.guard"` succeed in a clean interpreter. - O-patch: for every `setattr`, string-patch or `raising=False` target touched (incl. the 15 `current_principal` patches), `python -c "import importlib; m=importlib.import_module('<mod>'); getattr(m,'<name>')"` succeeds; list them in the PR body. - Security review (AD10): the code-review lane's security-profile verdict is recorded in the PR body with no unresolved CRITICAL/HIGH finding; `test_authz_policy.py` and the tenancy suites pass with test counts ≥ baseline. - AC6 Full gate green. — `./dc check full --gate-result --timeout 1500` on the committed, rebased tree → `GATE_RESULT status=pass` (includes `tools/check_context.py`). - AC7 Coverage not reduced (R6). — Before/after per touched suite: `pytest --collect-only -q | tail -1` → after ≥ before; cite both numbers in the PR body. - AC8 Docs pointers current (R7). — `git grep -nF 'app/authz.py' -- '*.md' '*.toml' '*.json' '*.yml' .github/CODEOWNERS` → 0 hits (except git-history-only prose); `python3 tools/check_context.py` → exit 0. - AC9 Delivered one PR per module (R4). — `jaah-forge pr view <n>` → diff touches only this module (+ callers, AD2 line, CODEOWNERS, tests, docs); merged into `develop` through the merge lane after `pull_request_enqueue` (not auto-merge), shown MERGED. - AC10 Availability on dev — the product still works for the operator after each PR deploys (no flag involved; refactor ships ON by deploy). — Watch **Jaah Deploy** for the merge commit succeed on GitHub; then open `https://app.dev.jaah.ai` in a browser signed in as the operator: Tasks list loads; open an epic and a child Task detail with no console errors; create a Task `[split-check P10]`, edit and save it, Start it and observe a fleet runner claim it, then delete it; Nodes page lists nodes; Credentials page lists credentials; Settings and Access/Groups pages load. Observe each succeeds as it did before the PR (no new 403). - AC12 Quality: the result is more navigable, not just shorter. Fails if a split produces circular-import workarounds (function-local imports added only to dodge cycles), duplicated code across new modules, or modules so thin (one trivial function) that a reader must open many files to follow one flow. — Code-review lane on the PR: `git grep -n "^\s\+from app\." <new files>` new function-local imports each justified in a comment; reviewer verdict recorded in the PR body.

Outcome

Split app/authz.py into 7 single-domain modules under app/authz/ with no shim. Merged as #6150 (69bb4156b) via the merge lane, and Jaah Deploy succeeded. Fixed one new develop caller (routes/data_residency.py). Dev smoke clean; skipped the create/start-a-task leg. Draft #7713 filed for develop's mypy red.

Pull request

Artifacts

Activity

  • Draft
  • Ready
  • Runner started
  • Runner ended (done)
  • Run
  • Runner started
  • Runner ended (done)