Skip to content

fix(cli): register first-run projects on Windows#1775

Open
Priyanchew wants to merge 2 commits into
mainfrom
fix/1766-windows-global-config
Open

fix(cli): register first-run projects on Windows#1775
Priyanchew wants to merge 2 commits into
mainfrom
fix/1766-windows-global-config

Conversation

@Priyanchew
Copy link
Copy Markdown
Collaborator

@Priyanchew Priyanchew commented May 10, 2026

Summary

  • Use OS home resolution for ~ expansion instead of relying on HOME.
  • Register auto-created first-run projects in the global config.
  • Compare global registry paths case-insensitively on Windows.

Fixes #1561.

Test plan

  • pnpm --filter @aoagents/ao-core test -- src/__tests__/global-config.test.ts
  • pnpm --filter @aoagents/ao-cli test -- __tests__/lib/path-equality.test.ts __tests__/commands/start.test.ts
  • pnpm --filter @aoagents/ao-core typecheck && pnpm --filter @aoagents/ao-core build && pnpm --filter @aoagents/ao-cli typecheck && pnpm --filter @aoagents/ao-cli build

Use OS home resolution and Windows-aware registry path comparisons so first-run config creation registers projects globally across platforms.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 10, 2026

Test Coverage Report

Metric Value
Lines covered 1371/1932
Lines not covered 561/1932
Overall coverage 71.0%
Per-file breakdown
File Coverage
packages/cli/src/commands/start.ts 866/1236 (70.1%)
packages/cli/src/lib/path-equality.ts 19/19 (100.0%)
packages/cli/src/lib/resolve-project.ts 182/309 (58.9%)
packages/core/src/global-config.ts 304/368 (82.6%)

Uncovered lines

  • packages/cli/src/commands/start.ts: L116-L117, L128-L130, L133-L135, L137, L139-L143, L146-L147, L149, L151-L155, L157-L159, L199-L200, L205-L223, L225-L232, L234-L235, L241-L247, L249, L280-L281, L295-L297, L306-L307, L310-L334, L379-L393, L395-L398, L400-L414, L442, L453, L477-L478, L481-L482, L498-L503, L541-L542, L566-L569, L596-L597, L604-L614, L617-L618, L642-L643, L646-L650, L658-L665, L672-L673, L710-L713, L728-L733, L750-L752, L754-L761, L763-L765, L767-L772, L774-L775, L869-L872, L919, L941-L949, L977-L985, L993-L997, L1006-L1008, L1041-L1042, L1058-L1059, L1065-L1066, L1068-L1069, L1073, L1128-L1129, L1147-L1152, L1185-L1186, L1192-L1193, L1264, L1284-L1285, L1369-L1370, L1422-L1439, L1496, L1530-L1532, L1555-L1562, L1589-L1597, L1608-L1609, L1643-L1644, L1652-L1663, L1666, L1682, L1688, L1691-L1699, L1709-L1710, L1751-L1754, L1757, L1759-L1761, L1766-L1767, L1776-L1780, L1782-L1783, L1792-L1793, L1803-L1808, L1844-L1850
  • packages/cli/src/lib/resolve-project.ts: L146-L158, L169-L172, L175-L186, L188-L189, L191-L205, L209-L212, L214-L216, L218-L229, L235-L245, L247-L260, L264-L265, L307-L309, L369-L370, L385, L389-L399, L443-L444, L446-L447, L462, L464-L466, L470-L477, L484-L485
  • packages/core/src/global-config.ts: L26, L38, L97, L353-L354, L376, L399, L419-L421, L424, L443, L450, L458-L459, L461, L464, L494-L496, L509-L511, L513-L515, L517-L518, L520-L522, L572, L589, L597-L603, L606, L633, L643, L692-L693, L705, L714, L853, L861, L950, L961, L963, L967, L969, L971, L975, L1063, L1096, L1108-L1109, L1165-L1167, L1176

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 10, 2026

Greptile Summary

This PR fixes three Windows-specific regressions in the first-run flow: ~ expansion now uses os.homedir() instead of the HOME env var (which is absent on Windows), auto-created projects are now registered in the global config (previously skipped), and global registry path comparisons are now case-insensitive on Windows.

  • homedir() substitution (path-equality.ts, resolve-project.ts, start.ts): three call sites updated consistently; the fix is minimal and correct.
  • Global registration on first run (start.ts): registerProjectInGlobalConfig is called inside a try/catch after the YAML is written, downgrading errors to a console warning so a path-collision or pre-existing entry never hard-fails the first-run flow.
  • registryPathCompareKey / registryPathsEqual (global-config.ts): new private helpers replace bare resolve(a) === b comparisons in registerProjectInGlobalConfig and isCanonicalGlobalConfigPath, adding realpathSync fallback and Windows lowercasing.

Confidence Score: 5/5

Safe to merge — all three Windows regressions are correctly addressed and the changes are well-contained.

All three fixes (homedir(), global registration, case-insensitive registry comparisons) are correct and the new try/catch in autoCreateConfig ensures registration failures never break the first-run flow. Tests cover the happy path and collision path. The only gap is a missing POSIX counterpart to the new Windows test, which is a coverage nit with no functional impact.

No files require special attention.

Important Files Changed

Filename Overview
packages/core/src/global-config.ts Introduces registryPathCompareKey/registryPathsEqual helpers using realpathSync + Windows lowercasing; replaces three bare resolve(a) === b comparisons in registerProjectInGlobalConfig and updates isCanonicalGlobalConfigPath.
packages/cli/src/commands/start.ts Adds registerProjectInGlobalConfig call in a try/catch after writing the first-run YAML, and switches tilde expansion in addProjectToConfig to homedir(). Registration failure is gracefully downgraded to a warning.
packages/cli/src/lib/path-equality.ts Replaces process.env[HOME] with homedir() for tilde expansion — the correct cross-platform fix.
packages/cli/src/lib/resolve-project.ts Single-line fix in fromPath: replaces process.env[HOME]
packages/core/src/tests/global-config.test.ts Adds a Windows-only test for isCanonicalGlobalConfigPath case-insensitive comparison. The guide requires a paired POSIX test, which is absent.
packages/cli/tests/commands/start.test.ts Two new tests cover global-registration on first run: one asserts successful registration, one asserts graceful downgrade on collision.
packages/cli/vitest.config.ts Adds a path alias for @aoagents/ao-plugin-runtime-process so the new start.test.ts cases can resolve the runtime plugin.

Sequence Diagram

sequenceDiagram
    participant User as User (ao start)
    participant AC as autoCreateConfig
    participant FS as Filesystem
    participant GC as registerProjectInGlobalConfig
    participant Registry as Global Config

    User->>AC: ao start (first run, no config)
    AC->>FS: writeFileSync(agent-orchestrator.yaml)
    FS-->>AC: OK
    AC->>GC: registerProjectInGlobalConfig(projectId, name, path, localConfig)
    GC->>GC: normalizeRegisteredProjectPath realpathSync + resolve
    GC->>Registry: withFileLockSync loadGlobalConfig
    Registry-->>GC: existing entries
    GC->>GC: registryPathsEqual resolve + realpathSync + lowercase on Windows
    alt No collision
        GC->>Registry: saveGlobalConfig atomic write
        GC-->>AC: effectiveProjectId
        AC->>User: Registered in global config
    else Path already registered
        GC-->>AC: throws Error
        AC->>User: Warning Could not register
    end
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/core/src/__tests__/global-config.test.ts:507-520
**Missing paired POSIX branch test for `registryPathCompareKey`**

`docs/CROSS_PLATFORM.md` requires that any new platform-branching code have both a Windows test and a POSIX test exercised on every CI host. The new Windows case (mocking `process.platform = "win32"`) is covered here, but there is no counterpart test that mocks `process.platform = "linux"` and asserts that `isCanonicalGlobalConfigPath` treats different-cased paths as distinct (POSIX is case-sensitive). Without it, a future change that accidentally lowercases on all platforms would pass CI silently.

Reviews (2): Last reviewed commit: "fix(cli): handle first-run registration ..." | Re-trigger Greptile

Comment thread packages/cli/src/commands/start.ts Outdated
Comment thread packages/core/src/global-config.ts
Warn when global registration collides after local config creation and canonicalize registry path comparisons through realpath when possible.
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.

ao start first-run via autoCreateConfig does not register project in global config

1 participant