Skip to content

refactor(core): split types.ts into co-located types/ domain modules (#2024 PR 1)#2026

Open
harshitsinghbhandari wants to merge 7 commits into
AgentWrapper:mainfrom
harshitsinghbhandari:refactor/core-types-domain-split
Open

refactor(core): split types.ts into co-located types/ domain modules (#2024 PR 1)#2026
harshitsinghbhandari wants to merge 7 commits into
AgentWrapper:mainfrom
harshitsinghbhandari:refactor/core-types-domain-split

Conversation

@harshitsinghbhandari
Copy link
Copy Markdown
Contributor

Part of #2024 — PR 1 of the phased packages/core/src restructure.

What

Decompose the 2094-line types.ts god-file into 16 co-located domain modules under types/, organized along the file's existing section banners. types.ts becomes a documented barrel that re-exports all of them.

types.ts            barrel (full architecture-map header) → types/
types/              session, runtime, agent, workspace, tracker, scm, notifier,
                    terminal, events, reactions, config, plugin, metadata,
                    services, errors, portfolio

Surface-preserving by design

This is a pure relocation — no API or behavior change:

  • index.ts is untouched (still export * from "./types.js").
  • The @aoagents/ao-core/types subpath export still resolves.
  • All 136 exported names are identical before/after (verified by diff).
  • The moved declarations are byte-for-byte identical to the originals (verified by reconstructing the old file from the 16 modules).

Build fix (the one non-mechanical change)

rollup.config.ts now derives its input from package.json exports instead of two hardcoded entries.

Why: the build uses preserveModules, which tree-shakes a pure re-export barrel (no runtime side effects) — index.ts's export * flattens straight to the underlying modules, so dist/types.js was never emitted. tsc still emits dist/types.d.ts, so typecheck passes while bundler/runtime resolution of the subpath export fails (caught by a Next.js/webpack build, not by tsc). Making every published entrypoint an explicit input prevents this — entry points are never tree-shaken. This also pre-empts the same break for the lifecycle-manager / session-manager / config barrels in later PRs.

Verification

  • pnpm build (full graph incl. web/Next.js) — green
  • pnpm typecheck (all packages) — green
  • pnpm test (core) — 1323/1323 pass
  • ESLint — clean
  • All 14 subpath-export dist/*.js targets present on disk

Commits

  1. refactor(core): split types.ts → types/ modules + barrel
  2. docs(core): expand the barrel header into a full architecture map
  3. fix(core): emit a dist chunk for every subpath export entrypoint

harshitsinghbhandari and others added 3 commits May 22, 2026 20:39
Decompose the 2094-line types.ts god-file into per-domain modules under
types/, organized along the existing section banners. types.ts becomes a
barrel re-exporting all modules, so the public surface (index.ts re-export
and the ./types subpath export) is unchanged. Pure move — no behavior or
API change. Part of ComposioHQ#2024 (PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restore and substantially extend the top-of-file documentation that the
2000-line god-file used to carry: the public-surface invariant (why this
barrel must stay at this path), the 8 plugin slots + core services
overview, a per-module map of what each types/ module owns, the
type-vs-runtime-value note, and editing conventions. Comment-only — no
API or behavior change. Part of ComposioHQ#2024 (PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drive rollup's input map from package.json `exports` so each published
"@aoagents/ao-core/<x>" entrypoint is an explicit entry point. Under
preserveModules, rollup tree-shakes pure re-export barrels (no runtime
side effects) — index's `export *` flattens to the underlying modules and
the barrel's own chunk (e.g. dist/types.js) is never emitted, breaking the
`./types` subpath export for bundlers/runtime even though tsc still emits
the .d.ts. Entry points are never tree-shaken, so listing every export
keeps barrel entrypoints emitted. Required for the types/ split (and the
lifecycle-manager/session-manager/config barrels in later PRs).

Part of ComposioHQ#2024 (PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

Greptile Summary

This PR decomposes the 2094-line types.ts god-file into 16 co-located domain modules under packages/core/src/types/, making types.ts a pure barrel that re-exports all of them. A companion fix to rollup.config.ts ensures every published package.json subpath export gets a real dist/*.js chunk by deriving rollup's input map from the exports field rather than hardcoding two entry points.

  • Type decomposition: all 136 previously exported names are preserved via export * from \"./types/<module>.js\" re-exports; cross-module references within types/ uniformly use import type to avoid runtime circular dependencies.
  • Rollup build fix: exportEntryPoints() reads package.json at config-file-relative path (via import.meta.url + fileURLToPath), mapping each \"./dist/X.js\" import condition to src/X.ts, so pure re-export barrels are no longer tree-shaken away before emit.
  • Test update: server-compatibility.test.ts correctly redirects its OrchestratorConfig source-text assertion from the old monolithic types.ts to the new types/config.ts location.

Confidence Score: 5/5

Safe to merge — this is a pure relocation of type declarations with no API, runtime, or behavioral changes.

Every moved declaration is byte-for-byte identical to the original; all cross-module references within types/ use import type (no runtime circular dependencies); the barrel re-exports all 16 modules preserving the same 136 exported names; the rollup fix correctly pins the package.json-to-source mapping via import.meta.url; and the one non-mechanical change (rollup config) is well-tested by a full pnpm build pass.

No files require special attention. The rollup.config.ts change is the most logic-bearing part of this PR and reads cleanly — path resolution is anchored to the config file via import.meta.url, and missing or malformed export entries produce visible warnings at build time.

Important Files Changed

Filename Overview
packages/core/rollup.config.ts Replaces two hardcoded input entries with exportEntryPoints(), which derives rollup inputs from package.json exports using import.meta.url-based path resolution; warns on exports lacking an import condition.
packages/core/src/types.ts Converted from a 2094-line god-file to a documented barrel that re-exports 16 domain modules; no declarations remain here.
packages/core/src/types/session.ts Session domain types extracted cleanly; cross-imports from runtime.js, agent.js, scm.js are all import type, avoiding runtime circular deps.
packages/core/src/types/agent.ts Agent plugin interface and related types extracted correctly; all sibling imports use import type.
packages/core/src/types/config.ts Configuration types extracted; correctly imports ObservabilityLevel from ../observability.js (parent src) rather than a sibling types module.
packages/core/src/types/errors.ts Error classes and detection helpers extracted with no imports, making this a dependency-free leaf module.
packages/core/src/types/services.ts Core non-pluggable service interfaces (SessionManager, LifecycleManager, PluginRegistry) and related option/result types extracted correctly with import type cross-references.
packages/web/server/tests/server-compatibility.test.ts Updated the OrchestratorConfig source-text assertion to read from types/config.ts instead of the monolithic types.ts, correctly tracking the moved declaration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["types.ts (barrel)"] --> B["types/session.ts"]
    A --> C["types/runtime.ts"]
    A --> D["types/agent.ts"]
    A --> E["types/workspace.ts"]
    A --> F["types/tracker.ts"]
    A --> G["types/scm.ts"]
    A --> H["types/notifier.ts"]
    A --> I["types/terminal.ts"]
    A --> J["types/events.ts"]
    A --> K["types/reactions.ts"]
    A --> L["types/config.ts"]
    A --> M["types/plugin.ts"]
    A --> N["types/metadata.ts"]
    A --> O["types/services.ts"]
    A --> P["types/errors.ts"]
    A --> Q["types/portfolio.ts"]
    B -. "import type" .-> C
    B -. "import type" .-> D
    B -. "import type" .-> G
    D -. "import type" .-> B
    D -. "import type" .-> C
    D -. "import type" .-> L
    D -. "import type" .-> M
    L -. "import type (non-sibling)" .-> R["../observability.ts"]
    R2["index.ts"] -->|"export * from './types.js'"| A
    S["package.json exports"] -->|"./types -> dist/types.js"| A
    T["rollup.config.ts"] -->|"exportEntryPoints()"| S
Loading

Reviews (3): Last reviewed commit: "refactor(core): warn on rollup export en..." | Re-trigger Greptile

Comment thread packages/core/rollup.config.ts
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Splits packages/core/src/types.ts into domain-focused modules under packages/core/src/types/ while keeping types.ts as the stable public barrel for @aoagents/ao-core/types. Updates the core Rollup build to emit a JS chunk for every published subpath export by deriving inputs from package.json#exports.

Changes:

  • Moved the former types.ts declarations into 16 co-located src/types/*.ts domain modules and converted src/types.ts into a documented re-export barrel.
  • Updated packages/core/rollup.config.ts to compute Rollup input from package.json exports so barrels like types.ts always produce dist/*.js.
  • Kept the external type surface stable via re-exports (no signature changes intended).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/core/src/types.ts Replaced the god-file with a documented barrel re-exporting types/ domain modules.
packages/core/src/types/session.ts Session lifecycle, activity, and session-related runtime constants/types.
packages/core/src/types/runtime.ts Runtime plugin slot types (runtime handle, attach info, metrics, create config).
packages/core/src/types/agent.ts Agent plugin slot types (launch config, session info, process probe helpers).
packages/core/src/types/workspace.ts Workspace plugin slot types (create/destroy/list/restore contracts).
packages/core/src/types/tracker.ts Tracker plugin slot types (issue CRUD + prompt/branch helpers).
packages/core/src/types/scm.ts SCM plugin slot types (PR/CI/review/webhook/batch enrichment contracts).
packages/core/src/types/notifier.ts Notifier plugin slot types (notifications + optional actions/posting).
packages/core/src/types/terminal.ts Terminal plugin slot types (human session opening semantics).
packages/core/src/types/events.ts Orchestrator event model (EventType/Priority + OrchestratorEvent).
packages/core/src/types/reactions.ts Reaction engine config/result types.
packages/core/src/types/config.ts Orchestrator + project configuration types and helpers.
packages/core/src/types/plugin.ts Plugin system contracts (slot/manifest/module + preflight intent).
packages/core/src/types/metadata.ts Persisted session metadata file schema.
packages/core/src/types/services.ts Core service interfaces (SessionManager/LifecycleManager/PluginRegistry).
packages/core/src/types/errors.ts Error helpers + exported error classes.
packages/core/src/types/portfolio.ts Cross-project portfolio aggregation types.
packages/core/rollup.config.ts Builds one Rollup entry point per package.json subpath export to ensure all dist/*.js targets are emitted.

Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/src/types.ts Outdated
harshitsinghbhandari and others added 3 commits May 22, 2026 21:30
The server-compatibility test reads the raw source of the OrchestratorConfig
interface to assert it has no `dataDir`. After the types.ts split that
interface lives in types/config.ts, so the test must read the new path.
Fixes the failing Test Web CI job. Part of ComposioHQ#2024 (PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Fix inaccurate per-module comments flagged in review: process-probe
helpers/PROCESS_PROBE_INDETERMINATE are in agent.ts (not runtime.ts),
normalizeAgentPermissionMode is in config.ts (not agent.ts), and the
kill/claim/cleanup option & result types are in services.ts (metadata.ts
holds only SessionMetadata). Part of ComposioHQ#2024 (PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Address review: the export-entrypoint reader used a CWD-relative
readFileSync("package.json"), which throws ENOENT if rollup is invoked
from a directory other than the package root. Pin the path to the config
file via fileURLToPath(new URL("./package.json", import.meta.url)) —
fileURLToPath (not URL.pathname) keeps it correct on Windows. Part of
ComposioHQ#2024 (PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@harshitsinghbhandari
Copy link
Copy Markdown
Contributor Author

Addressed the CI failure and review feedback:

  • CI (Test Web)server/__tests__/server-compatibility.test.ts reads the raw source of the OrchestratorConfig interface to assert it has no dataDir. That interface moved to types/config.ts in the split, so the test now reads the new path. Verified locally: pnpm --filter @aoagents/ao-web test → 84 files / 992 tests pass. (e5b4e952)
  • @greptile-apps P2 (rollup CWD-relative read) — pinned the package.json read to the config file via fileURLToPath(new URL("./package.json", import.meta.url)). Used fileURLToPath rather than URL.pathname since this repo is Windows-first-class and .pathname yields /C:/… there. Rebuild confirms dist/types.js still emits. (4cd0b9b7)
  • @copilot (inaccurate barrel annotations) — corrected: process-probe helpers / PROCESS_PROBE_INDETERMINATE are in agent.ts (not runtime.ts); normalizeAgentPermissionMode is in config.ts (not agent.ts); kill/claim/cleanup option & result types are in services.ts (metadata.ts holds only SessionMetadata). (82261e6d)

Note: the test-path change touches packages/web, so this PR isn't strictly core/src-only — but that test introspects core's source layout, so it had to follow the move.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

All three items look good — thanks for the thorough follow-through.

CI fix (e5b4e952) — the test now reads types/config.ts directly. One thing worth noting for future maintainability: this test hard-codes a relative path three directories up (../../../core/src/types/config.ts). If the types/ layout changes again in a later PR, the test will silently break in the same way it did here. A comment like // Update this path if OrchestratorConfig moves or a // Part of #2024 note would make the coupling explicit and easier to catch in future refactors. Not a blocker, just a suggestion.

Rollup CWD fix (4cd0b9b7)fileURLToPath(new URL("./package.json", import.meta.url)) is exactly the right pattern. The Windows rationale in the commit message is correct and the implementation is solid.

Barrel annotation fix (82261e6d) — the updated comments match the actual module contents: agent.ts for process-probe, config.ts for normalizeAgentPermissionMode, services.ts for kill/claim/cleanup types. The barrel is accurate now.

LGTM — happy to approve once CI passes.

@harshitsinghbhandari
Copy link
Copy Markdown
Contributor Author

@greptile-apps can you approve PRs??

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 22, 2026

No, I can't approve PRs. I can submit reviews with COMMENT or REQUEST_CHANGES events, but the APPROVE action is restricted to human reviewers with write access to the repository. You'll need a team member to formally approve this one.

…tion

Address review: exportEntryPoints() silently skipped any package.json
exports entry lacking an { import } field (e.g. a future string-form or
require-only export), which would drop its dist emit with no signal. Now
it warns with the entry name at build time. Part of ComposioHQ#2024 (PR 1).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@harshitsinghbhandari
Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. How each point was handled:

  • Duplicate PREnrichmentData / BatchObserver in scm.ts — agreed it's pre-existing (verified: same duplication exists on main at types.ts lines 906/1110 and 935/1141; this PR relocated them verbatim). Keeping it out of this PR to preserve the byte-for-byte / unchanged-surface guarantee. Filed as follow-up: refactor(core): remove duplicate PREnrichmentData / BatchObserver interface declarations #2028.
  • exportEntryPoints() silently skipping non-{import} entries — fixed in c…: it now console.warns with the entry name instead of silently dropping the dist emit, which also covers a future string-form / require-only export. Build re-verified, dist/types.js still emitted.
  • No try/catch around the package.json read — intentionally left as-is. If a package's own package.json is missing/malformed during its build, the build is already non-viable and Node's native JSON error is actionable; wrapping it adds little for a build-config script. Happy to add a guard if you'd prefer.

No changes to the type modules or public surface in this round — only the rollup warning. CI is green across Linux + Windows.

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.

2 participants