fix(core): handle wrapped config session names for dot project paths

[ChiragArora31]

Resolves #1521.

Summary

• Sanitizes the legacy wrapped storage key component generated for project paths like ..
• Adds a spawn regression that verifies dot-path projects use the valid V2 runtime/tmux session name (app-1).

Root cause

Legacy wrapped config handling used basename(projectPath) directly, so path: . could produce a storage key suffix of ..

Approach

Resolve the project path relative to the config directory and normalize unsafe storage-key characters. Current V2 runtime naming remains prefix-based and is covered by the new spawn test.

Verification

• pnpm --filter @aoagents/ao-core test -- project-resolver.test.ts session-manager/spawn.test.ts
• pnpm --filter @aoagents/ao-core typecheck
• pnpm --filter @aoagents/ao-core build
• pnpm --filter @aoagents/ao-core test was also run; it still has existing timeout failures in migration-storage-v2.test.ts and orchestrator-prompt.dist.test.ts.

[greptile-apps]

Greptile Summary

This PR hardens session prefix and storage-key generation for project paths that collapse to invalid identifiers (., .., /). It introduces sanitizeIdentifierComponent and deriveSessionPrefixFromProjectPath in paths.ts, threads them through the wrapped-config storage-key builder, applyProjectDefaults, getRegisteredSessionPrefix, and the migration pre-flight checker so all code paths apply the same sanitization logic.

• sanitizeIdentifierComponent + deriveSessionPrefixFromProjectPath: new primitives in paths.ts that strip filesystem punctuation from any basename/path fragment and produce a stable, tmux-safe prefix even when basenames collapse to the fallback (\"project\"), using a path-fingerprint to disambiguate.
• generateLegacyWrappedStorageKey (exported): now resolves the project path relative to the config directory before extracting the basename, avoiding the bare-. suffix that was the root cause, and appends a path fingerprint when multiple risky paths would otherwise share the same sanitized component.
• extractProjectPrefixes in migration: updated to re-derive prefixes from the project path when the stored sessionPrefix is invalid, matching the runtime\u2019s new behavior and fixing the active-session-detection gap called out in a previous review.

Confidence Score: 4/5

The PR is safe to merge and correctly addresses the dot-path session naming bug across all three affected code paths.

All three previously-flagged code paths have been consistently updated. One gap remains in applyProjectDefaults where relative project paths resolve from CWD rather than the config directory, creating a potential inconsistency with the storage-key resolution that correctly anchors to the config directory.

packages/core/src/config.ts — the applyProjectDefaults function resolves relative project paths from CWD rather than the config directory; requires a minor signature change to fully align with the storage-key logic.

Important Files Changed

Filename	Overview
packages/core/src/paths.ts	Adds sanitizeIdentifierComponent and deriveSessionPrefixFromProjectPath; threads sanitization into generateSessionPrefix. Logic is correct and backward-compatible for normal project names.
packages/core/src/config.ts	Exports generateLegacyWrappedStorageKey and switches applyProjectDefaults to deriveSessionPrefixFromProjectPath. Storage key now correctly resolves relative to config dir; session prefix still resolves relative to CWD (see comment).
packages/core/src/global-config.ts	Introduces getRegisteredSessionPrefix that re-derives invalid stored prefixes from the project path; used consistently in findSessionPrefixOwner, registerProjectInGlobalConfig, and resolveProjectIdentity.
packages/core/src/migration/storage-v2.ts	Updates extractProjectPrefixes to validate stored session prefixes with sanitizeIdentifierComponent before using them, fixing the active-session-detection gap for projects with invalid stored prefixes.
packages/core/tests/config.test.ts	Adds three regression tests for dot-path storage key sanitization, fingerprint disambiguation, and collision avoidance across risky paths. Tests cover the fixed code path directly via the exported function.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[project path e.g. '.'] --> B[sanitizeIdentifierComponent]
    B --> C{sanitized == 'project'?}
    C -- No --> D[generateSessionPrefix basename]
    C -- Yes --> E[SHA-256 fingerprint of resolved path]
    E --> F[generateSessionPrefix 'x-fingerprint']
    D --> G[tmux-safe sessionPrefix]
    F --> G

    A --> H[generateLegacyWrappedStorageKey]
    H --> I[resolve relative to configDir]
    I --> J[sanitizeIdentifierComponent basename]
    J --> K{component == 'project'?}
    K -- No --> L[hash-component]
    K -- Yes --> M[path fingerprint]
    M --> N[hash-project-fingerprint]

    P[stored sessionPrefix in global config] --> Q[getRegisteredSessionPrefix]
    Q --> R{sanitizeIdentifierComponent stored == stored?}
    R -- Yes --> S[use stored prefix]
    R -- No --> T[deriveSessionPrefixFromProjectPath entry.path]
    T --> G

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/config.ts:600-602
When `applyProjectDefaults` calls `deriveSessionPrefixFromProjectPath(project.path)` with a relative path like `"."`, `resolve()` inside that function anchors to the process CWD — not the config file's directory. `generateLegacyWrappedStorageKey` (called earlier in the load pipeline) correctly resolves relative to `configDir`. If the caller's CWD differs from the config directory (e.g. running `ao` from `/home/user/` against a config at `/repos/myapp/agent-orchestrator.yaml`), the two calls resolve `"."` to different directories, producing a storage key fingerprinted on `/repos/myapp/` but a session prefix derived from `/home/user/` — and the prefix silently changes each time the CWD changes.

```suggestion
    if (!project.sessionPrefix) {
      project.sessionPrefix = deriveSessionPrefixFromProjectPath(
        resolve(configDir, project.path),
      );
    }
```

_{Reviews (3): Last reviewed commit: "fix(core): sanitize migration active-ses..." | Re-trigger Greptile}

[i-trytoohard]

Review: fix(core): handle wrapped config session names for dot project paths

Verdict: The fix in config.ts is correct. The test needs rework.

✅ The Fix (config.ts) — Looks Good

The sanitizeStorageKeyComponent function and the resolve() before basename() are both correct fixes for the reported bug:

1. resolve(configDir, projectPath) — resolves . relative to the config directory before taking basename, so path: "." becomes the actual directory name instead of .. This is the right approach.
2. sanitizeStorageKeyComponent() — belt-and-suspenders sanitization that replaces invalid chars with -, strips leading/trailing dashes, and falls back to "project" if the result is still invalid. Clean implementation.

The storage key ${hash}-${basename} feeds into paths.ts:generateSessionId() which formats it as ${storageKey}-${prefix}-${num} — that composite string gets validated against /^[a-zA-Z0-9_-]+$/, so a bare . would explode. The fix prevents that.

❌ The Test (spawn.test.ts) — Does Not Exercise the Fix

Greptile's comment is correct and I'm confirming it independently. The test is not a valid regression test:

1. sessionPrefix: "app" is hardcoded in setupTestContext() (line 323). The spread ...project copies it. The test project always has sessionPrefix: "app" regardless of path.
2. The wrapped config path is never triggered. classifyConfigShape() reads the YAML from disk, but the test constructs an in-memory config and passes it through validateConfig(). The YAML file contains projects: {}, so applyWrappedLocalStorageKeys processes the parsed YAML (which has no projects), not the in-memory config.
3. Session ID "app-1" is determined by sessionPrefix: "app" alone. Even if generateLegacyWrappedStorageKey produced hash-., the prefix derivation at line 586 checks if (!project.sessionPrefix) — since "app" is set, the storage key is irrelevant.

The test would pass identically on the pre-patch code. It does not test what it claims.

How to Fix the Test

Option A (preferred): Unit-test sanitizeStorageKeyComponent and generateLegacyWrappedStorageKey directly:

describe("generateLegacyWrappedStorageKey", () => {
  it("sanitizes dot path to valid component", () => {
    const key = generateLegacyWrappedStorageKey(configPath, ".");
    expect(key).toMatch(/^[a-zA-Z0-9_-]+$/);
    expect(key).not.toContain(".");
  });
});

This requires exporting the functions or testing via internals.

Option B: Construct a config without sessionPrefix, write a real wrapped YAML to disk, and load it through loadConfig() for a full end-to-end test.

Minor Notes

• The regex and sanitizer are top-level but only used in one place. Fine — small and self-contained.
• Fallback to "project" is reasonable but worth a brief inline comment explaining when it triggers (e.g. path resolving to empty string).

[ChiragArora31]

Addressed both review concerns in d1df661a:

• Reworked the regression to directly exercise the wrapped storage-key fix via generateLegacyWrappedStorageKey.
• Hardened applyProjectDefaults to sanitize path-derived sessionPrefix before generating the prefix, so dot-path projects cannot produce invalid .-style tmux names.

Validation: pnpm --filter @aoagents/ao-core test -- __tests__/config.test.ts src/__tests__/session-manager/spawn.test.ts (103 passing).

[i-trytoohard]

Re-review after test rework

Verdict: LGTM. Ready to merge. ✅

Changes since last review

• Removed the bad spawn test that didn't exercise the fixed path
• Replaced with a proper config-level test in config.test.ts that:
  • Writes a real YAML with path: . to disk
  • Loads it through loadConfig() (full pipeline — classify → applyWrappedKeys → validate → applyDefaults)
  • Calls generateLegacyWrappedStorageKey directly and asserts no . in the result
  • Asserts sessionPrefix is also sanitized (valid regex, not bare .)
• Exported generateLegacyWrappedStorageKey for direct testing — clean, minimal change
• Added defense-in-depth: sanitizeStorageKeyComponent now also guards the sessionPrefix derivation path (line 595), not just the storage key path. Good call — basename(".") → "." would also blow up in generateSessionPrefix → prefix collision detection.

CI

All checks passing — Test, Typecheck, Lint, Web Tests, Integration Tests, Fresh Onboarding. Green across the board.

Nice fix, Chirag. Ship it.

[ashish921998]

We dont care about backward compatibility, right now

[harshitsinghbhandari]

Nice iteration on the test after the first round — the rewrite that drives loadConfig end-to-end and exports generateLegacyWrappedStorageKey is exactly right, and hardening applyProjectDefaults was a good catch. Just one thing before merge:

The same bug still reproduces through global-config.ts. The PR sanitizes the basename at two sites (config.ts:86, config.ts:595), but generateSessionPrefix(basename(...)) is still called unsanitized in three places:

• global-config.ts:593 — getRegisteredSessionPrefix, used by prefix-collision detection
• global-config.ts:711 — registration flow (ao project add)
• global-config.ts:806 — resolveProjectIdentity

paths.ts:65 short-circuits on length <= 4 and returns the input verbatim, so generateSessionPrefix(".") → ".". Anyone going through the global-config flow with a dot path still gets an invalid tmux prefix.

Per CLAUDE.md ("WE DONOT APPLY SURFACE FIXES. WE FIX THE CORE FIRST"), the cleanest move is to sanitize inside generateSessionPrefix itself — move sanitizeStorageKeyComponent to paths.ts and call it on entry. That fixes all five present call sites and any future ones by construction, and the rename to something like sanitizeIdentifierComponent falls out naturally (since it's no longer storage-key-specific).

Two smaller things:

• Fallback collision. sanitizeStorageKeyComponent("") returns "project". Two projects in the same wrapped config that both sanitize-empty (path: "/", "..") collide on ${hash}-project. Realistic? No. Worth a Zod-time guard or a ${hash}-project-${id} fallback? Probably yes.
• Tests. Worth pinning path: "/", path: "..", and the collision case while you're in there.

No changeset either — the storage-key format change is user-visible (even if, as @ashish921998 noted, the old layout was never functional, so no migration needed).

Once generateSessionPrefix is hardened at the core, this should be ready.

[ChiragArora31]

Thanks @harshitsinghbhandari — implemented the core-first approach you suggested.

What changed

• Added sanitizeIdentifierComponent in paths.ts and made generateSessionPrefix sanitize at the start so every caller (including global-config.ts and migration) gets safe prefixes automatically.
• Added deriveSessionPrefixFromProjectPath for cases where the basename collapses to the generic project token after sanitization, so we don’t get duplicate pro-style prefixes when two projects legitimately use nasty paths like / vs .. in the same YAML.
• Legacy wrapped generateLegacyWrappedStorageKey now uses the shared sanitizer and appends a short path fingerprint when the sanitized basename is that generic fallback (so / vs .. don’t share the same ${hash}-project dir).
• Added regression tests for /, fingerprint disambiguation, and prefix collision avoidance; added the requested changeset.

Merged latest main into this branch and re-ran pnpm --filter @aoagents/ao-core test -- src/__tests__/paths.test.ts __tests__/config.test.ts + typecheck.

Happy to tweak naming (sanitizeIdentifierComponent) if you prefer something even more general.

[harshitsinghbhandari]

This is great — verified end-to-end:

• sanitizeIdentifierComponent in paths.ts and generateSessionPrefix sanitizing at entry means every caller (current and future) is safe by construction. Exactly the core-first fix.
• All three global-config.ts call sites updated, plus the migration/storage-v2.ts site I hadn't flagged. Nice catch.
• deriveSessionPrefixFromProjectPath + the storage-key fingerprint suffix close the / vs .. collision cleanly.
• Tests in paths.test.ts and config.test.ts pin the sanitizer behavior, prefix collision avoidance, and the legacy-storage-key fingerprint regex ^[a-f0-9]{12}-project-[a-f0-9]{8}$.
• Changeset is there.

Non-blocking: getRegisteredSessionPrefix (global-config.ts:593) and resolveProjectIdentity (:807) still trust entry.sessionPrefix verbatim when set, and GlobalProjectEntrySchema.sessionPrefix is just z.string().optional() (no regex). Any existing user who previously ran ao project add with a dot path has sessionPrefix: "." literally stored in their global config, and that stored value keeps flowing through. Since @ashish921998 already waived BC, this is fine to defer — but a one-liner that re-derives the prefix when the stored value fails the identifier regex would close it for free if you feel like adding it. Otherwise, ship it.