Integrate Project model into loader and decompose into composable stages

[ryancbahan]

WHY are these changes introduced?

PR #7022 introduced the Project and ActiveConfig domain models, but nothing consumed them (the loader still ran its own filesystem discovery, constructed AppConfigurationState as an intermediate, and threaded the entire previous AppInterface through reloads). This PR wires the new models into the loading pipeline and uses that as leverage to decompose the monolithic loadApp into composable stages with narrow interfaces.

WHAT is this pull request doing?

Decomposes the loader into three explicit stages:

• getAppConfigurationContext(dir, configName) → {project, activeConfig} — discovery only, no parsing or state construction
• loadAppFromContext({project, activeConfig, specifications, …}) → AppInterface — validation and assembly, for callers that already hold a Project
• loadApp({directory, configName, …}) — thin wrapper composing the two above

[Interim state] Replaces previousApp with narrow ReloadState:

Instead of threading the entire AppLinkedInterface through reloads (just to read 2 fields), reloadApp now constructs a ReloadState with only extensionDevUUIDs: Map<string, string> and previousDevURLs. This is a step change away from broad state passing/mutation, but needs more consideration on "permanent home" as we continue to decompose functionality.

Updates all consumers:

• linkedAppContext uses activeConfig.isLinked and activeConfig.file.content directly instead of going through AppConfigurationState
• link() returns {remoteApp, configFileName, configuration} — drops state from its return type
• use() reads activeConfig.file.content instead of calling loadAppConfiguration
• loadConfigForAppCreation uses activeConfig and project.directory directly

Removes dead code (-400 lines):

AppConfigurationState, AppConfigurationStateBasics, toAppConfigurationState, loadAppConfigurationFromState, loadAppUsingConfigurationState, loadAppConfiguration, getAppConfigurationState, getAppDirectory, loadDotEnv, loadHiddenConfig, findWebConfigPaths, loadWebsForAppCreation, getConfigurationPath (de-exported)

How to test your changes?

npx vitest run packages/app/src/cli/models/app/loader.test.ts
npx vitest run packages/app/src/cli/services/app-context.test.ts
npx vitest run packages/app/src/cli/services/app/config/link.test.ts
npx vitest run packages/app/src/cli/services/app/config/use.test.ts
npx vitest run packages/app/src/cli/services/context.test.ts
npx vitest run packages/app/src/cli/models/project/

Measuring impact

• n/a

Checklist

• I've considered possible cross-platform impacts (Mac, Linux, Windows)
• I've considered possible documentation changes

🤖 Generated with Claude Code

[ryancbahan]

• Remove packageManager, nodeDependencies, usesWorkspaces from App #7035
• Integrate Project model into loader and decompose into composable stages #7023 👈 (View in Graphite)
• Add Project domain model for filesystem abstraction #7022
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

Coverage report

St.❔	Category	Percentage	Covered / Total
🟢	Statements	82.17%	14936/18177
🟡	Branches	74.51%	7375/9898
🟢	Functions	81.3%	3774/4642
🟢	Lines	82.56%	14124/17108

Test suite run success

3939 tests passing in 1514 suites.

Report generated by 🧪jest coverage report action from fe0b46f

[github-actions]

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

[github-actions]

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

• Some seemingly private modules might be re-exported through public modules.
• If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/themes/api.d.ts

@@ -5,7 +5,6 @@ export type ThemeParams = Partial<Pick<Theme, 'name' | 'role' | 'processing' | '
 export type AssetParams = Pick<ThemeAsset, 'key'> & Partial<Pick<ThemeAsset, 'value' | 'attachment'>>;
 export declare function fetchTheme(id: number, session: AdminSession): Promise<Theme | undefined>;
 export declare function fetchThemes(session: AdminSession): Promise<Theme[]>;
-export declare function findDevelopmentThemeByName(name: string, session: AdminSession): Promise<Theme | undefined>;
 export declare function themeCreate(params: ThemeParams, session: AdminSession): Promise<Theme | undefined>;
 export declare function fetchThemeAssets(id: number, filenames: Key[], session: AdminSession): Promise<ThemeAsset[]>;
 export declare function deleteThemeAssets(id: number, filenames: Key[], session: AdminSession): Promise<Result[]>;

packages/cli-kit/dist/public/node/themes/theme-manager.d.ts

@@ -8,8 +8,8 @@ export declare abstract class ThemeManager {
     protected abstract removeTheme(): void;
     protected abstract context: string;
     constructor(adminSession: AdminSession);
-    findOrCreate(name?: string, role?: Role): Promise<Theme>;
-    fetch(name?: string, role?: Role): Promise<Theme | undefined>;
+    findOrCreate(): Promise<Theme>;
+    fetch(): Promise<Theme | undefined>;
     generateThemeName(context: string): string;
     create(themeRole?: Role, themeName?: string): Promise<Theme>;
 }
\ No newline at end of file

[isaacroldan]

---

Review assisted by pair-review

[isaacroldan]

The reload behaviour has changed, let's fix that first 🙏

I think some extensive tophat of app dev is necessary before merging, even it appears to work, we need to check the manifest.json and the bundle uploaded to ensure it includes everything

[ryancbahan]

Pulling context from Slack -- we discovered the reload issue with manifest writes is already on main, not introduced in this pr. I've added more tests here and will tophat thoroughly + document.

[ryancbahan]

this was hogging up memory now that tests are sharded

[ryancbahan]

Added a bunch more tests around hot reload, file watching, and config switching. Doing a bunch of local tophatting and good so far.

[dmerand]

I just have a couple of extra comments.

[dmerand]

extensionFilesForConfig() / webFilesForConfig() intentionally reduce configured globs to simple path prefixes, so patterns like foo/*/extensions over-match. This PR is where that simplification becomes production loader behavior: loadConfigForAppCreation(), loadWebs(), and createExtensionInstances() now route through those helpers instead of the old direct glob discovery. The new integration tests cover rescans and reloads, but they do not protect nontrivial glob shapes. Can we either preserve true glob semantics here or add regression coverage for complex extension_directories and web_directories patterns before merging?

[ryancbahan]

I'll update to preserve original behavior and add test.

[dmerand]

This test now only proves that discovery errors propagate out of use(). If the intended contract is still "don’t cache a config unless it is valid enough to be used as the active app config," then I think we need at least one fixture-based test with a parseable-but-schema-invalid TOML and an assertion that the cached preference is not updated.

If the new contract is intentionally "syntactically valid + has client_id is enough for use()," then I’d suggest updating the test name/comments to make that boundary explicit, because the old test shape implied a stronger guarantee.

[ryancbahan]

I'll make the test more explicit that syntactic validation is the aim here.

[dmerand]

I see the intent here is to make app config use only switch the active TOML and defer full schema validation to later commands. I’m still worried about the UX regression relative to the old path: this command used to reject configs that were parseable TOML but no longer valid app config, whereas now it can cache them as the active selection as long as client_id is present. That shifts the failure away from the point where the user made the choice, which makes diagnosis harder.

If that behavior change is intentional, could we call it out explicitly in the command contract/tests? Otherwise I think we should keep a narrow validated load here before writing the cached preference.

[ryancbahan]

Can you elaborate on which diagnoses are specifically harder? The tests and jsdoc call out the update behavior pretty clearly already.

[isaacroldan]

This makes sense to do, but I don't think it should be the file-watcher responsibility :thinking_face:
Maybe something to do at the start of dev, before the dev session starts?

[ryancbahan]

I'm not sure I agree -- the file watcher is what breaks if chokidar can't resolve dirs properly.

in any case, is there any way to decouple this feedback to unblock this pr? I think there are improvements we can continue to work on. This was addressing a pre-existing bug and I'd love to see us ship the project model if we agree it's the right abstraction.

[isaacroldan]

We can problably do this in a separate PR yeah. You are right this is fixing a pre-existing bug and is not related to the main goal of the PR.

[ryancbahan]

@dmerand @isaacroldan I'd love to get alignment on your full feedback of what it will take to get this merged. Does the current state of your comments reflect that? I just want to make sure there's clarity in next steps to getting this over the line, if we're in agreement that it's the right abstraction.

[isaacroldan]

if we know the activeConfig is linked, why not store the client_id in a way that can be accesed safely?
I know that when building the active config we are already validating this, but if that ever changes this would silently start failing.

A couple options:
- store the client_id as optional directly in the "ActiveConfig"
- Have something like this in ActiveConfig:

{
...
linkedStatus: {isLinked: false} | {isLinked: true, clientId: string}
...
}

[ryancbahan]

To me, this goes against the principle of what we are trying to do. We should be moving away from storing so much state and passing it around, and moving towards more derived state expressly at the callsite that needs it. This is the callsite at which we derive the state.

[isaacroldan]

I can see that point, but being linked is tied to having a clientId, so I don't think is that bad.

If not, I'd say we'll need to validate here that activeConfig.file.content.client_id is actually a non-empty string, not just casting.

not blocking anymore