Skip to content
This repository was archived by the owner on Apr 4, 2026. It is now read-only.

refactor: A2 path restructure — remove python/ prefix#28

Merged
Nafania merged 12 commits into
mainfrom
upstream/a2-path-restructure
Mar 29, 2026
Merged

refactor: A2 path restructure — remove python/ prefix#28
Nafania merged 12 commits into
mainfrom
upstream/a2-path-restructure

Conversation

@Nafania
Copy link
Copy Markdown
Owner

@Nafania Nafania commented Mar 29, 2026

Summary

Removes the python/ prefix from all module directories, aligning with upstream v1.1 structure:

  • python/helpers/helpers/
  • python/tools/tools/
  • python/api/api/
  • python/extensions/extensions/python/
  • python/websocket_handlers/websocket_handlers/

3649 import references updated. All 2559 tests pass (including integration).

Part of

Upstream backport project (Phase 1) — see docs/specs/2026-03-28-upstream-backport-design.md.

Changes

  • Directory movesgit mv preserving history
  • Import rewrites — all python.helpers.*helpers.*, etc.
  • Dynamic path loadersextension.py, agent.py tool loading, run_ui.py handler loading, websocket_namespace_discovery.py
  • get_base_dir() fix — adjusted for new directory depth
  • __init__.py files — created for all new top-level packages
  • Pytest config — added pythonpath = ["."] and --import-mode=importlib to resolve tests/helpers/ vs helpers/ collision
  • CI path triggers — updated for new directory structure
  • Documentation — AGENTS.md architecture diagram, all docs updated

Breaking Changes

  • All worktree branches require rebase after merge
  • All open PRs require rebase after merge
  • Any external code importing python.helpers.* etc. must update imports

Test plan

  • All 2559 unit + integration tests pass
  • Zero remaining python.(helpers|tools|api|extensions|websocket_handlers) references
  • Import smoke test from new paths
  • CI path triggers updated
  • AGENTS.md and all docs updated

Made with Cursor

@Nafania
Copy link
Copy Markdown
Owner Author

Nafania commented Mar 29, 2026

Code Review

Reviewed all 523 changed files across 9 commits (base 6fa12269..head 2e1de4c4).

Strengths

  • Zero stale references — all 3649 python.* dot-imports fully rewritten, no stragglers in .py or .yml files
  • Symmetric diff — 3866 insertions / 3866 deletions in source files, confirming pure rename with no accidental content changes during bulk sed
  • get_base_dir() correctly updated — navigates 1 level up from helpers/files.py instead of 2 from python/helpers/files.py
  • pytest config properly handles tests/helpers/ vs helpers/ shadowing via --import-mode=importlib and pythonpath = ["."]
  • All __init__.py files present — all 6 new top-level packages have them
  • CI path triggers properly updated — both push and pull_request filters list new directories
  • Dynamic loaders updatedrun_ui.py, websocket_namespace_discovery.py, agent.py tool loading all use correct new paths
  • Well-structured commit history — logical progression: docs → moves → imports → strings → fixups → docs

Issues

Important: Extension override paths silently broken

helpers/extension.py:33call_extensions() now passes subpaths to get_paths() in a way that breaks agent-profile and user extension overrides:

# Current (after this PR)
paths = subagents.get_paths(agent, "python", extension_point, default_root="extensions")

This produces:

  • ✅ Default path: extensions/python/<ext_point> (correct)
  • ❌ Agent profile: agents/<profile>/python/<ext_point> (should be agents/<profile>/extensions/python/<ext_point>)
  • ❌ User override: usr/python/<ext_point> (should be usr/extensions/python/<ext_point>)

No crash occurs — non-existent paths are silently skipped via must_exist_completely=True — but override mechanisms are silently dropped. Evidence: agents/_example/extensions/agent_init/ exists on disk but would be ignored by the new code.

Fix: Pass "extensions", "python" as separate subpath components:

paths = subagents.get_paths(agent, "extensions", "python", extension_point, default_root="")

Minor: USER_EXTENSIONS_FOLDER is dead code

helpers/extension.py:11 — the constant USER_EXTENSIONS_FOLDER = "usr/extensions" is defined but never used at runtime (only imported by test for assertion).

Minor: Test coverage gap for path resolution

tests/helpers/test_extension.py:65 — the test mocks subagents.get_paths entirely, so the actual subpath arguments are never verified. A targeted assertion would have caught the Important issue above:

@pytest.mark.asyncio
async def test_call_extensions_passes_correct_paths(self):
    with patch("helpers.subagents.get_paths", return_value=[]) as mock_gp:
        await call_extensions("agent_init", agent=None)
    mock_gp.assert_called_once_with(
        None, "extensions", "python", "agent_init", default_root=""
    )

Assessment

Ready to merge: With fixes (Important issue #1)

The refactoring is exceptionally clean for its scale — 523 files, zero stale references, symmetric diffs, well-structured commits, proper test infrastructure updates. The single substantive issue is a path regression in call_extensions() where agent-profile and user extension override paths silently changed. This is a one-line fix. After correcting it, this is ready for production.

Nafania added 10 commits March 29, 2026 15:06
Covers full backport plan: A2 path restructure, A1 plugin system,
A3-A8 architectural changes, and 50+ feature/fix items in parallel waves.

Made-with: Cursor
9 tasks with bite-sized steps: directory moves, automated import
rewriting, dynamic path updates, test verification, and PR creation.

Made-with: Cursor
python/helpers/ → helpers/
python/tools/ → tools/
python/api/ → api/
python/extensions/ → extensions/python/
python/websocket_handlers/ → websocket_handlers/
python/__init__.py removed

Imports will be fixed in next commit.

Made-with: Cursor
- Add __init__.py to top-level packages (helpers/, tools/, api/,
  extensions/, extensions/python/, websocket_handlers/)
- Add tests/__init__.py to prevent test subdirectories from shadowing
  top-level packages in pytest's module resolution
- Add pythonpath=["."] and --import-mode=importlib to pyproject.toml
- Fix helpers/rfc_files.py get_abs_path() going two levels up instead
  of one after the python/ directory removal
- Fix remaining hardcoded "python/" file paths in test files
  (test_context.py, test_metrics_collector.py, test_browser.py,
  test_memory_feedback.py)
- Fix test_code_execution_tool time.time mock using infinite iterator
  to avoid StopIteration from background threads calling time.time()

All 2559 tests pass (1 skipped, 0 failures).

Made-with: Cursor
get_paths() splices subpaths into agent-profile, user, and default
paths. With subpaths=("python", ext_point), override paths like
agents/<profile>/python/<ext_point> were wrong — missing "extensions"
segment. Fix: pass ("extensions", "python", ext_point) as subpaths
with empty default_root.

Made-with: Cursor
@Nafania Nafania force-pushed the upstream/a2-path-restructure branch from 2e1de4c to dfc2ee1 Compare March 29, 2026 12:09
@Nafania
Copy link
Copy Markdown
Owner Author

Nafania commented Mar 29, 2026

Follow-up Review (after fix commit dfc2ee1)

The fix commit addressed the original review's Important issue, but the fix is still incorrect. Extension override paths remain broken.

Analysis

get_paths() in helpers/subagents.py:300-359 applies *subpaths uniformly to ALL scopes (profile, user, project, default). There is no way to have python/ appear only in the default path using a single get_paths() call.

Pre-A2 (correct, working):

paths = subagents.get_paths(agent, "extensions", extension_point, default_root="python")
# subpaths = ("extensions", ext_point)
# → Profile:  agents/<profile>/extensions/<ext_point>  ✅
# → User:     usr/extensions/<ext_point>                ✅
# → Default:  python/extensions/<ext_point>             ✅

Commit 4944ee0 (broken):

paths = subagents.get_paths(agent, "python", extension_point, default_root="extensions")
# subpaths = ("python", ext_point)
# → Profile:  agents/<profile>/python/<ext_point>       ❌
# → User:     usr/python/<ext_point>                    ❌
# → Default:  extensions/python/<ext_point>             ✅

Commit dfc2ee1 (still broken):

paths = subagents.get_paths(agent, "extensions", "python", extension_point, default_root="")
# subpaths = ("extensions", "python", ext_point)
# → Profile:  agents/<profile>/extensions/python/<ext_point>  ❌
# → User:     usr/extensions/python/<ext_point>               ❌
# → Default:  extensions/python/<ext_point>                   ✅

Evidence: agents/_example/extensions/agent_init/_10_example_extension.py exists on disk. The fix looks for agents/_example/extensions/python/agent_init/ — the example agent's extension override is silently ignored.

Correct Fix

Split the path construction using the existing include_default=False parameter:

paths = subagents.get_paths(
    agent, "extensions", extension_point,
    default_root="", include_default=False,
)
default_path = files.get_abs_path("extensions", "python", extension_point)
if files.exists(default_path):
    paths.append(default_path)

This produces:

Scope Path Correct?
Project agent <proj>/.a0proj/agents/<profile>/extensions/<ext_point>
Project <proj>/.a0proj/extensions/<ext_point>
User agent usr/agents/<profile>/extensions/<ext_point>
Default agent agents/<profile>/extensions/<ext_point> ✅ (matches agents/_example/extensions/agent_init/)
User usr/extensions/<ext_point> ✅ (backward compatible)
Default extensions/python/<ext_point> ✅ (new post-A2 location)

Override priority is preserved: profile extensions come first (higher priority), default extensions come last. The merge logic in call_extensions() keeps the first occurrence per filename, so overrides correctly shadow defaults.

Assessment

Not ready to merge — the extension override regression is still present. The correct fix is a 4-line change in helpers/extension.py:33.

get_paths() applies *subpaths uniformly to all scopes. Since
default extensions moved to extensions/python/ but profile/user
overrides stay at agents/<profile>/extensions/<ext_point>, a single
call can't produce both paths correctly.

Split: use get_paths(include_default=False) for profile+user scopes
with subpaths=("extensions", ext_point), then manually append the
default path with the python/ segment.

Made-with: Cursor
@Nafania
Copy link
Copy Markdown
Owner Author

Nafania commented Mar 29, 2026

Review #3 — Fix Verified (commit d95c6d54)

Fix Verification

The split-path approach is correct. Full path trace for call_extensions("agent_init", agent) with profile _example:

Step 1 — get_paths(agent, "extensions", "agent_init", default_root="", include_default=False):

Priority Scope Path Correct?
1 Project + Profile <proj>/.a0proj/agents/_example/extensions/agent_init
2 Project <proj>/.a0proj/extensions/agent_init
3 User Profile usr/agents/_example/extensions/agent_init
4 Default Profile agents/_example/extensions/agent_init
5 User Global usr/extensions/agent_init

No python/ segment in override paths. Backward compatible with existing agent profiles.

Step 2 — Manual default path:

Priority Scope Path Correct?
6 (last) Default extensions/python/agent_init

Override priority preserved — the dedup logic (first-occurrence wins) ensures profile extensions shadow defaults.

Verified on disk:

  • agents/_example/extensions/agent_init/_10_example_extension.py — exists, would be found at priority 4 ✅
  • extensions/python/agent_init/ — exists, appended at priority 6 ✅

Other get_paths() Callers — No Similar Issues

Caller Assessment
agent.py:682 (parse_prompt) subpaths=("prompts",) — no python/ involved. OK
agent.py:689 (read_prompt) Same. OK
agent.py:1046 (tool loading) subpaths=("tools", name+".py"), default_root=""python/ was a prefix, not sandwiched. OK
skills.py:51 subpaths=("skills",) — never used python/. OK
_15_load_profile_settings.py:13 Profile/project only, include_default=False. OK

Websocket handlers use get_abs_path("websocket_handlers") directly — independent of get_paths().

Stale Reference Scan

  • from python. / import python.0 matches
  • "python/" / 'python/' string paths — 0 matches
  • default_root="python"0 matches

Minor (Nice to Have)

  1. Test coverage gaptest_extension.py patches get_paths entirely, never asserting on include_default=False or the manual default path append. A regression to the old single-call pattern wouldn't be caught by tests. Consider adding a targeted assertion.

  2. Dead constantsDEFAULT_EXTENSIONS_FOLDER and USER_EXTENSIONS_FOLDER in helpers/extension.py are defined but never referenced in production code (pre-existing, not introduced by this PR).

Assessment

Ready to merge: Yes

The split-path fix correctly addresses the fundamental incompatibility between override paths (no python/ segment) and the default path (python/ sandwiched between extensions/ and <ext_point>). All other get_paths() call sites are unaffected. Zero stale references remain. Override priority is preserved. Clean for production.

Remove DEFAULT_EXTENSIONS_FOLDER and USER_EXTENSIONS_FOLDER — unused
in production code (pre-existing dead code).

Add test_call_extensions_splits_override_and_default_paths to verify
get_paths is called with include_default=False and the default path
(extensions/python/<ext_point>) is appended separately. Prevents
regression to the single-call pattern.

Made-with: Cursor
@Nafania Nafania merged commit c287238 into main Mar 29, 2026
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant