♻️ refactor(arch): make ArchUnitTS architecture suite pass - #394
Open
cteyton wants to merge 14 commits into
Open
♻️ refactor(arch): make ArchUnitTS architecture suite pass#394cteyton wants to merge 14 commits into
cteyton wants to merge 14 commits into
Conversation
Restore the architecture-tests package that was stranded on an unmerged branch, rebased cleanly onto current main so it runs against today's code (the recipes -> commands rename is picked up automatically since packages are discovered from the filesystem). - packages/architecture-tests: ArchUnitTS suite (boundaries, cross-domain, domain-purity, layering rules) with a dedicated jest.arch.config.ts and an `arch` Nx target, kept off `nx run-many -t test` on purpose - scripts/build-arch-tsconfig.mjs: generates the self-contained tsconfig.arch.json ArchUnitTS needs (was missing on main) - package.json: `test:arch` script + `archunit` devDependency - .gitignore: ignore generated tsconfig.arch.json - eslint.config.mjs: extend the jest-config override to jest.*.config.ts Run with `pnpm run test:arch`. The suite currently surfaces pre-existing architecture violations on main; triaging/fixing those is deliberately out of scope for this commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The I*DelayedJobs interfaces referenced concrete delayed-job classes in application/jobs/, so keeping them under domain/jobs/ made the domain layer depend on the application layer (hexagonal violation caught by the arch domain-purity rule). Move them into application/jobs/ next to the jobs they describe, for commands, deployments and git. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two hexagonal violations in coding-agent: - domain/repository/ICodingAgentDeployer.ts imported the concrete DefaultSkillsDeployResult from infra (domain -> infra). Move the port and its result types (DefaultSkillMetadata, DefaultSkillsDeployResult, DeployDefaultSkillsOptions) into @packmind/types as the canonical contract; the domain now re-exports them and infra imports them from types. This also drops the ICodingAgentDeployer = unknown placeholder in the registry contract. - infra PackmindDeployer imported StandardsIndexService from application (infra -> application). The service is a pure index-string builder, so move it next to PackmindDeployer under infra/repositories/packmind/. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CommandService and CommandVersionService declared their repository ports as interfaces but defaulted them to `new CommandRepository()` / `new CommandVersionRepository()`, which made the application layer import the concrete infra repositories (hexagonal violation). CommandsServices already injects the repositories from the ICommandsRepositories aggregator, so drop the concrete defaults and imports and require the ports to be injected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The git application layer (GitAdapter, AddGitProviderUseCase, UpdateGitProviderUseCase, validateProviderCredentials) imported the GithubAppMode type from the concrete infra GithubTokenResolverFactory, coupling application to infra. GithubAppMode is a pure contract, so hoist it to @packmind/types and repoint the factory, GitHexa and the git barrel at it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
LlmAdapter (application) instantiated the concrete AIProviderRepository from the DataSource, coupling the application layer to infra. Add an ILlmRepositories port and an LlmRepositories infra aggregator that owns the DataSource-to-repo wiring; LlmHexa builds the aggregator and injects it, and the adapter resolves the repository through the port only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RenderPackageAsPluginUseCase (deployments) instantiated the concrete ClaudePluginDeployer from @packmind/coding-agent, and DeployDefaultSkillsUseCase, DownloadSkillZipForAgentUseCase and DefaultSkillsMetadataEnricher imported coding-agent types directly — both cross-domain violations. Add an ICodingAgentPort.renderPackageAsClaudePlugin port (contract in @packmind/types), implemented by CodingAgentAdapter delegating to CodingAgentRepositories where the ClaudePluginDeployer now lives. deployments calls the port and imports the shared types (ICodingAgentDeployer, DefaultSkillMetadata) from @packmind/types instead of coding-agent source. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PackageRepository (deployments/infra) imported CommandSchema, StandardSchema, SkillSchema and SpaceSchema from other domains to hydrate full artefact entities and resolve org spaces — a cross-domain violation. Move entity hydration up to PackageService, which now resolves spaces via ISpacesPort and batch-fetches commands/standards/skills through their ports; the repository only reads its own aggregate and junction tables (findBySlugsAndSpaceIds returning artefact IDs). Add the batch reads the hydration needs: AbstractRepository.findByIds (shared), ICommandsPort.getCommandsByIdsInternal, IStandardsPort.getStandardsByIds and ISkillsPort.getSkillsByIds, each wired through the domain adapter/service/repo. DeploymentsAdapter injects the ports into PackageService once the HexaRegistry has resolved them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Greptile SummaryThis PR introduces and enables an ArchUnitTS suite while refactoring package boundaries to satisfy its 23 architecture rules.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains within the scope of the previous review threads. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
API[apps/api] --> Ports["@packmind/types ports"]
Ports --> Adapters[Domain adapters]
Adapters --> UseCases[Use cases]
UseCases --> Services[Application services]
Services --> RepoPorts[Repository interfaces]
RepoImpl[Infrastructure repositories] --> RepoPorts
Arch[ArchUnitTS suite] -. enforces boundaries .-> API
Arch -. enforces boundaries .-> Adapters
Arch -. enforces boundaries .-> Services
Arch -. enforces boundaries .-> RepoImpl
Reviews (4): Last reviewed commit: "🔧 chore(arch): drop PACKMIND_EDITION fr..." | Re-trigger Greptile |
Wire the ArchUnitTS suite (packages/architecture-tests, `test:arch`) into the automated checks, mirroring the original architecture-tests branch: - .husky/pre-push: run `architecture-tests:arch` after the frontend typecheck so a push fails fast if any architecture rule breaks (Nx-cached, no-op when nothing relevant changed). - build.yml: add a blocking `architecture-tests` job to the Build and Test workflow. - architecture.yml: add a dedicated non-blocking (continue-on-error) workflow on push/PR that reports architecture violations without gating merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The `arch` Nx target derives its @packmind/* alias map from
tsconfig.paths.{oss,proprietary}.json (via select-tsconfig ->
tsconfig.base.effective.json -> tsconfig.arch.json), but neither
paths file was declared as a cache input. Editing an alias within
the same edition left every input unchanged, so Nx replayed the
last passing result and the suite could miss newly-forbidden
imports (false green). Add both paths files as inputs.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pass an explicit compare function to Array.prototype.sort() so package discovery ordering is reliable and locale-aware, resolving typescript:S2871. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The README documented a "Domain-event location" rule via src/events.arch.spec.ts and counted the suite at 24 rules, but that spec does not exist and the suite is 23 rules. Trim the docs to match the implemented scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set PACKMIND_EDITION to ${{ vars.PACKMIND_EDITION }} in both the tsconfig
generation and the run step, matching build.yml, so the standalone
architecture workflow tests the configured edition instead of being pinned
to oss.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



What
Gets the
architecture-testsArchUnitTS suite to 23/23 passing (was 7 failing across 3 suites). Each violation is fixed as its own commit; every fix keeps the affected packages building, testing, and linting green (verified via the pre-pushnx affectedrun).The fixes follow the hexagonal boundaries the suite enforces — no shortcuts/back-edges, domains talk only through
@packmind/typesports — usingclaude/sharp-goodall-pkw8nxas the design reference (adapted from its olderrecipesnaming to the currentcommandsnaming).Fixes
4b25f44I*DelayedJobsfromdomain/jobs/toapplication/jobs/(commands, deployments, git) — they reference application job classes3f92181ICodingAgentDeployer(+ result types) to@packmind/types; moveStandardsIndexServiceinto infra besidePackmindDeployer3cbcff1= new CommandRepository()defaults)20208b6GithubAppModetype to@packmind/types; git consumers import it from theredd61c03LlmRepositoriesaggregator soLlmAdapterreaches the repo through a port7f1baa9ICodingAgentPort.renderPackageAsClaudePlugin;ClaudePluginDeployernow lives inside coding-agent7ff81dePackageRepositoryno longer imports other domains' EntitySchemas — artefact hydration moves toPackageServicevia batch ports (getCommandsByIdsInternal/getStandardsByIds/getSkillsByIds+ sharedAbstractRepository.findByIds)Notes
getStandardsByIdsenriched standards with a versionsummary, but the current schema has nosummaryfield onStandard/StandardVersion, so that variant is simplified to mirror the skills/commands pattern (no version lookup).🤖 Generated with Claude Code