Skip to content

refactor(workspace): decompose clone & install cluster into sub-packages (#4988) - #5023

Merged
Trecek merged 12 commits into
developfrom
impl-workspace-clone-installed-decomposition-20260912-130741
Sep 13, 2026
Merged

Trecek merged 12 commits into
developfrom
impl-workspace-clone-installed-decomposition-20260912-130741

Conversation

@Trecek

@Trecek Trecek commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

Relocates 11 files from workspace/ top level into two new sub-packages, per #4988's requirement to bring workspace/ under Tier-1's 20-file cap (currently 32):

  • workspace/clone/ (public sub-package, clone/worktree lifecycle): clone.py -> clone/__init__.py (the facade; its content becomes the package __init__.py directly since Python resolves the package unconditionally over a same-named sibling file), clone_registry.py -> clone/_registry.py, cleanup.py -> clone/_cleanup.py, worktree.py -> clone/_worktree.py, _clone_detect.py -> clone/_detect.py, _clone_remote.py -> clone/_remote.py.
  • workspace/_installed/ (Tier-2 private sub-package, install/lease/artifact authority): _install_state.py -> _installed/_state.py, _installed_artifact.py -> _installed/_artifact.py, _projection_cache.py -> _installed/_projection_cache.py, _shared_asset_store.py -> _installed/_shared_asset_store.py, _update_obligation.py -> _installed/_update_obligation.py.

workspace/__init__.py now imports each cluster through one facade instead of six/three separate absolute-path blocks. _projected_artifact/'s shards (authority.py, _hook_repair.py, _publication.py, _generation_publication.py) are repointed to the new _installed submodule paths directly, not through its facade. No backward-compatibility shims at the old paths, per this repo's own rule.

Forced decomposition (scope addition, approved mid-implementation)

Two files this move puts over REQ-CNST-010's diff-scoped 750-non-import-line cap (both already over the cap before this ticket) required decomposition, since tests/arch/_acceptance_policy_surfaces.py's exemption ledger is explicitly human-authored-only:

  • _installed/_projection_cache.py (812 lines) -> extracts the public-asset inventory and cache-key record into a new sibling _installed/_projection_assets.py, re-imported back so every existing consumer keeps resolving unchanged (639 / 188 non-import lines).
  • _projected_artifact/_generation_publication.py (810 lines) -> extracts the stale-generation reconciliation state machine into a new sibling _generation_prune.py, reached via TYPE_CHECKING-only for GenerationArtifactRetirementOwner to keep the publication -> prune dependency one-way (531 / 298 non-import lines).

Both splits were researched and adversarially reviewed by separate subagent passes before implementation; review caught and this PR fixes several would-be runtime bugs the initial proposals missed (a missing from __future__ import annotations, a missing StrEnum import that would fail at class-definition time, and a missing logger that would NameError on the first non-empty prune pass) -- verified by directly exercising the previously-broken call paths, not just import-checking the modules.

Commits

  1. Foundational source restructure (both cluster moves + the two forced decompositions + the two full-tree-scanning registry scripts that must move in the same commit)
  2. Arch/registry test file updates (file-count limits, ambient-home, flock inventory, temp-cleanup-debt, origin-isolation, session-binding)
  3. tests/_retention_surface.py repointed, including 7 line-shifted AUDITED_RETENTION_DECISIONS keys computed from the actual final file
  4. 26 white-box test files repointed to the new import paths
  5. 4 additional registries found by adversarial review (outside the plan's originally-scoped test directories)
  6. Stale documentation corrections

Verification

  • pre-commit run --all-files clean on every commit (ruff format/check, mypy, all custom architectural gates -- including the file-length and policy-relaxation gates this change specifically had to satisfy)
  • Every plan verification grep re-run clean against the final committed state (zero stale references to any of the 11 old paths across src/, tests/, scripts/, docs/)
  • Real Python import checks across the whole workspace package, plus functional smoke tests directly exercising the two runtime bugs adversarial review caught (confirmed fixed, not just import-clean)
  • git log --follow confirms rename history preserved through all 11 moves
  • Import-linter contract check: 12 kept, 0 broken (confirms IL-0/IL-1/IL-2/IL-3 layering is intact)
  • Full task test-all was attempted via the test_check tool but hit a 15-minute infrastructure timeout before completing (not a test failure -- only ~29 of the suite's tests had executed by the time it was signal-terminated); this PR's CI run will provide the authoritative full-suite result

🤖 Generated with Claude Code

Trecek added a commit that referenced this pull request Sep 13, 2026
…efining submodules

Follows the tests/execution/test_process_submodules.py pattern for an analogous
decomposition. Guards against silent drift if a facade symbol's defining
submodule changes without the __init__.py export being updated to match.

Addresses the DISCUSS-recommended (info/tests) finding from PR #5023 review-pr
round 0 asking for a test pinning each facade symbol to its real location.
Trecek and others added 12 commits September 12, 2026 18:55
…ges (#4988)

Relocates 11 files from workspace/ top level into two new sub-packages,
per #4988's requirement to bring workspace/ under Tier-1's 20-file cap
(currently 32):

- workspace/clone/ (public sub-package, clone/worktree lifecycle):
  clone.py -> clone/__init__.py (facade, content becomes the package init
  since Python resolves the package unconditionally over a same-named
  sibling file), clone_registry.py -> clone/_registry.py, cleanup.py ->
  clone/_cleanup.py, worktree.py -> clone/_worktree.py, _clone_detect.py ->
  clone/_detect.py, _clone_remote.py -> clone/_remote.py.

- workspace/_installed/ (Tier-2 private sub-package, install/lease/artifact
  authority): _install_state.py -> _installed/_state.py,
  _installed_artifact.py -> _installed/_artifact.py,
  _projection_cache.py -> _installed/_projection_cache.py,
  _shared_asset_store.py -> _installed/_shared_asset_store.py,
  _update_obligation.py -> _installed/_update_obligation.py.

workspace/__init__.py now imports each cluster through one facade instead
of six/three separate absolute-path blocks. _projected_artifact/'s shards
(authority.py, _hook_repair.py, _publication.py, _generation_publication.py)
are repointed to the new _installed submodule paths per Design Decision 6:
they reach _installed's submodules directly, not through its facade.

No backward-compatibility shims at the old paths, per this repo's own rule.

Also decomposes two files this move puts over REQ-CNST-010's diff-scoped
750-non-import-line cap (both already over the cap before this ticket;
decomposing them is the only path available to an automated session, since
tests/arch/_acceptance_policy_surfaces.py is explicitly human-authored-only):

- _installed/_projection_cache.py (812 lines) -> extracts the public-asset
  inventory and cache-key record (is_projected_asset,
  iter_public_plugin_asset_files, per_file_asset_digest,
  public_plugin_asset_digest, ProjectionCacheKey,
  PROJECTION_CACHE_KEY_EXCLUSIONS) into a new sibling
  _installed/_projection_assets.py, re-imported back so every existing
  consumer keeps resolving unchanged (639 / 188 non-import lines, verified
  against tests.arch._line_budget.count_budget_lines).

- _projected_artifact/_generation_publication.py (810 lines) -> extracts
  the stale-generation reconciliation state machine (_is_selected_generation,
  _GenerationPruneDisposition, _generation_residue_managed_path,
  _revalidate_generation_mutation_target, _quarantine_invalid_generation,
  _resume_generation_residue, _reconcile_generation_candidate,
  _log_generation_prune_reconcile) into a new sibling
  _generation_prune.py, reached via TYPE_CHECKING-only for
  GenerationArtifactRetirementOwner to keep the publication -> prune
  dependency one-way (531 / 298 non-import lines).

Both decompositions were researched and adversarially reviewed by separate
subagent passes before implementation; the review caught and this commit
fixes several would-be runtime bugs the initial proposals missed (a missing
`from __future__ import annotations`, a missing StrEnum import that would
fail at class-definition time, and a missing logger that would NameError on
the first non-empty prune pass) -- verified here by directly exercising the
previously-broken call paths, not just import-checking the modules.

Also updates scripts/check_ambient_home_boundary.py and
scripts/check_launch_path_totality.py's own path registries in this same
commit -- both are full-tree-scanning pre-commit hooks that fail hard
(not just on the touched files) the moment any file in the diff moves, so
their fixes cannot be deferred to a later commit.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
…4988)

Updates the architectural guard registries that hardcode relative source
paths for the 11 files relocated in the prior commit, plus the new
_generation_prune.py shard the forced decomposition created:

- test_subpackage_isolation_file_counts.py: FILE_COUNT_LIMITS["workspace"]
  32 -> 21 (32 minus the 11 moved files). No new entries for
  "workspace/clone" or "workspace/_installed": clone/ genuinely lands in the
  walk's dirs_to_check and passes the 6-file default-10 comparison;
  _installed/ (leading underscore) is skipped by the walk entirely, exactly
  mirroring the pre-existing workspace/_projected_artifact.
- test_ambient_home_boundary.py: mirrors check_ambient_home_boundary.py's
  registry (already updated in the prior commit) exactly.
- test_hook_flock_nonblocking.py: 10 flock-inventory tuples repointed across
  the 6 moved modules plus the 2 tuples for functions the decomposition
  moved into _generation_prune.py.
- test_shell_capture_trust_anchor.py, test_python_no_hardcoded_temp.py:
  path-keyed temp-cleanup-debt and hardcoded-temp-whitelist entries for
  worktree.py/clone_registry.py/clone.py -> their new clone/ paths.
- test_origin_isolation_contract.py: _clone_remote.py's PYTHON_ALLOWLIST
  entries repointed to _remote.py -- this one matches by basename, and
  unlike _clone_detect.py (directory-only move), _clone_remote.py's
  basename itself changes, so these entries need updating despite the
  basename-matching logic.
- test_session_binding_single_authority.py: one function-local import
  repointed to the new _installed._projection_cache path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
)

Updates every RECLAIMER_TARGETS/RECLAIMER_CONVERGENCE_CASES/
ACKNOWLEDGED_NON_RECLAIMERS entry and alias constant keyed on the 5 moved
files with a retention-audit footprint (clone.py, clone_registry.py,
worktree.py, _install_state.py, _projection_cache.py), plus the one entry
the forced _generation_publication.py decomposition also touches
(_reconcile_generation_candidate moves to _generation_prune.py).

Also renumbers the 7 line-pinned AUDITED_RETENTION_DECISIONS keys for
prune_stale_generations (L811/L824/L832/L834/L836/L839/L842 ->
L529/L542/L550/L552/L554/L557/L560): removing the 8 symbols that moved into
_generation_prune.py shifts every statement below them in the retained
file. Computed by reading the actual post-decomposition file directly
rather than hand-reconstructing the shift, per two independent adversarial
reviews that both warned hand-computation is fragile (one produced two
different plausible-looking answers depending on one implementation
choice).

grep -rn "workspace/clone_registry.py\|workspace/worktree.py\|workspace/clone.py\|workspace/_install_state.py\|workspace/_projection_cache.py" tests/_retention_surface.py
returns zero hits after this change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
… modules (#4988)

Applies the "preserve-binding-style, relocate-path" rule to every direct
import/patch of the 11 relocated modules across 26 test files: each import
form (from-import, aliased import, dotted patch string) keeps its exact
binding shape, only the dotted/relative module path segment changes.

Notable non-mechanical cases:
- test_cleanup.py: `from autoskillit.workspace import CleanupResult,
  _delete_directory_contents` split into two lines -- the private
  underscored name is no longer reachable as a workspace-package attribute
  now that cleanup.py moved into clone/_cleanup.py; workspace/__init__.py
  imports only the public delete_directory_contents alias.
- test_tools_clone.py: `from autoskillit.workspace import clone_registry`
  repointed to `from autoskillit.workspace.clone import _registry as
  clone_registry`, matching the same rebind server/_misc.py needed (prior
  commit) since clone_registry.py no longer sits at the workspace top level
  to bind the import-side-effect attribute.
- test_projection_cache_key.py's `_PUBLIC_PLUGIN_ASSET_NAMES` private import
  repoints to the new sibling _installed/_projection_assets.py directly
  (the forced decomposition's genuine new home for that constant), rather
  than re-importing it back into _projection_cache.py where it would sit
  unused and fail ruff's F401 check.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
These four registries sit outside tests/workspace/, tests/contracts/, and
tests/arch/ -- the plan's originally-scoped file list -- and were missed
in initial review before adversarial re-review caught them:

- test_plugin_source_ratchets.py: REGISTRY_READ_ALLOWLIST (2 entries),
  PLUGIN_MUTATION_ALLOWLIST (5 tuples), STRICT_PLUGIN_WRITE_ALLOWLIST
  (1 entry) repointed to the new _installed/ paths.
- test_state_lock_contract.py: _FCNTL_ALLOWED_RELATIVE_PATHS entry for
  clone_registry.py -> clone/_registry.py. This registry has no .exists()
  guard before read_text(), so a stale entry would raise an uncaught
  FileNotFoundError rather than a clean assertion failure.
- test_schema_read_convention.py, test_schema_version_convention.py: one
  path-keyed entry each, repointed with line numbers unaffected (pure
  renames, content at the cited lines is untouched).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
…4988)

No test enforces these; fixed for accuracy since it's the same change that
made them stale:

- core/types/_type_constants_retirements.py: a code comment above
  RETIRED_INSTALL_ARTIFACT_SHAPES naming the old _install_state.py path.
- test_git.py: a docstring naming the old workspace.worktree module path,
  updated to the public facade spelling (workspace.clone) since that's the
  form external callers actually use.
- docs/glossary.md, docs/safety/workspace.md, docs/version-pipeline.md:
  backtick-quoted mentions of clone_registry.py/_install_state.py's old
  paths.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
#4988)

/audit-impl caught three additional stale-path references to workspace/
clone.py that the original Step 9.3 pass missed (it only covered
docs/glossary.md:33, docs/safety/workspace.md:25, and docs/version-
pipeline.md:128 -- distinct locations in the same two files, plus one in
a test docstring Step 9 never scoped):

- docs/glossary.md:26 ("clone-based isolation" entry, separate from the
  "clone registry" entry at line 33 already fixed)
- docs/safety/workspace.md:9 ("Clone-based isolation" section, separate
  from the "Clone registry" section at line 25 already fixed)
- tests/contracts/test_instruction_surface.py:346 (a docstring naming the
  old module path; the test's own code -- `import autoskillit.workspace
  .clone as clone_mod` -- was already correct, since workspace.clone still
  resolves to the package facade unchanged)

grep -rn "workspace/clone\.py" docs/ tests/contracts/test_instruction_surface.py
returns zero hits after this change.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
…d of reaching into private _registry submodule

Restores clone_registry as documented public surface of the clone sub-package
(it was public workspace-facade surface before the #4988 decomposition moved
the module to the private _registry.py). server/_misc.py now imports it from
the clone package's public boundary instead of importing the private _registry
submodule directly and re-aliasing it locally with a narrative comment.

Addresses reviewer findings: IL-3->private-IL-1 reach (arch), undocumented
alias breaking the leading-underscore convention (cohesion), and the
backward-compat-hack alias comment (slop).
…odule docstring

Keeps the purpose statement and the durable one-way-dependency /
TYPE_CHECKING-only invariants; drops the "split out of X" framing per the
slop dimension finding.
… facade

test_req_imp_007_server_cli_no_unauthorized_cross_submodule_imports and
test_no_cross_package_submodule_imports both require IL-3 server code to
import only from the top-level autoskillit.workspace facade, never reach
into a workspace sub-package (autoskillit.workspace.clone) directly. The
prior fix exposed clone_registry from workspace.clone but had _misc.py
import it from there; this re-exports it one layer up through
workspace/__init__.py and restores the exact pre-refactor import path
(`from autoskillit.workspace import clone_registry as clone_registry`).
…efining submodules

Follows the tests/execution/test_process_submodules.py pattern for an analogous
decomposition. Guards against silent drift if a facade symbol's defining
submodule changes without the __init__.py export being updated to match.

Addresses the DISCUSS-recommended (info/tests) finding from PR #5023 review-pr
round 0 asking for a test pinning each facade symbol to its real location.
Four architectural-guard failures surfaced by the full suite, all root-caused
to registries/facades not updated for the new workspace/clone/ and
workspace/_installed/ sub-packages:

- Add missing CLAUDE.md adapter shims for the two new sub-packages'
  AGENTS.md guides (workspace/clone/, workspace/_installed/) — the
  guide/adapter sibling contract requires every non-root AGENTS.md to have
  an exact "@AGENTS.md" CLAUDE.md sibling.
- Add workspace/clone/__init__.py to test_ast_rules.py's _EXEMPT_INITS
  (P14-2 architectural-debt allowlist). Design Decision 1 requires this
  facade to hold clone.py's functions directly rather than double-indirect
  through a nested module, which the pure-facades guard would otherwise
  reject; this is the same sanctioned exemption already used by 5 other
  business-logic __init__.py files.
- Correct stale line numbers in tests/_retention_surface.py's
  AUDITED_RETENTION_DECISIONS for _reconcile_projection_entry,
  prune_stale_projections (workspace/_installed/_projection_cache.py, all
  shifted -175 by the Addendum A _projection_assets.py split) and
  _enqueue_legacy_installed_plugin_versions (workspace/_installed/_state.py,
  shifted +1). Recomputed by scanning the actual post-move ASTs rather than
  hand-calculating the shift, per the plan's own Addendum A precedent.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mg5MHeZ9xiEKjbsMVUABvD
@Trecek
Trecek force-pushed the impl-workspace-clone-installed-decomposition-20260912-130741 branch from bae853a to afe0950 Compare September 13, 2026 02:05
@Trecek
Trecek added this pull request to the merge queue Sep 13, 2026
Merged via the queue into develop with commit 6a77cc4 Sep 13, 2026
8 checks passed
@Trecek
Trecek deleted the impl-workspace-clone-installed-decomposition-20260912-130741 branch September 13, 2026 02:36
Trecek added a commit that referenced this pull request Sep 13, 2026
Resolves four conflicts against develop (#5023 clone/install cluster,
#5025 session_skills package, version bump to 0.10.1133):

- tests/arch/test_ast_rules.py: union both _EXEMPT_INITS entries
  (develop's workspace/clone/__init__.py + our skill_capabilities facade).
- tests/arch/test_subpackage_isolation_file_counts.py: workspace/ ceiling
  recomputed to 1 — develop took 24->6 (clone/install + session_skills
  moves), this branch removes the last 5 flat capability modules. Verified
  against the actual top-level file count, which develop's new
  test_session_skills_package_shape asserts exactly.
- src/autoskillit/workspace/AGENTS.md: keep our skill_capabilities/ package
  paragraph; drop the _shared_asset_store.py section, which develop
  relocated to workspace/_installed/AGENTS.md (keeping it would duplicate).
- .autoskillit/test-source-map.json: resolved as a pure 5-key path rename
  preserving each entry's test list and the provenance envelope, mirroring
  the convention #5025 used for its own 6-key session_skills rename. Avoids
  a regeneration that would both absorb unrelated cross-PR drift and stamp
  pytest_exit_code=1 from the pre-existing hooks failure.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant