[Orsted290]

问题定位

原逻辑可能把 Codex 的 sessions 目录误当成 CLI 可执行文件位置。Codex 安装在非默认目录、GUI 进程的 PATH 不完整，或 Windows npm 安装只提供 codex.cmd 时，Hook 启动会报 ENOENT。另有路径别名导致 Hook 信任匹配失败，以及安装失败后状态残留、sessions 目录消失后无法卸载的问题。

修复方式及代码改动

• 在 AgentSourceCore/src/codex-executable.ts 增加统一 CLI 解析：优先使用显式配置，其次查找 Codex 插件目录、PATH、平台安装目录和 npm 包内原生可执行文件；验证文件存在后才返回，避免将 sessions 路径或 Windows 命令 shim 当成可执行程序。
• 在 AgentSourceCore/src/codex-installation.ts 统一路径规范化：解析符号链接；处理 Windows 大小写、UNC 和 extended-path 表示；按规范化后的 Codex Home 串行化安装与卸载。
• 在 App/backend/.../codex/{hook-trust,target}.ts 和 Memory/.../codex/{hook-trust,target}.ts 同步修复两条 Codex 集成路径：先写完安装文件，再请求 Codex 信任；保存受管理文件的完整快照，后续步骤失败时恢复文件、目录和链接。
• 在 Memory/src/agent-source/runtime.ts 区分安装与卸载：POST 仍要求检测到 Codex；DELETE 即使 sessions 已消失也能清理 Hook 并更新连接状态。
• 补充路径解析、规范化、失败回滚、并发安装/卸载和删除后的反向测试。

Windows 验收（非原生机器）

• win32 路径单测覆盖非默认目录、带引号或大小写不同的 PATH、npm shim 对应的原生二进制、LocalAppData 安装位置及显式 CLI 配置。
• Wine 11 / Rosetta 兼容层验收：验证了非默认 Windows Codex 路径发现；安装后 app-server 发现并信任 5 个 Memmy Hook；卸载后新 app-server 返回 0 个 Memmy Hook。
• 兼容层在 Hook 子进程/回写阶段发生崩溃，因此完整 Codex 对话采集未验证；上述结果不代表 Windows 原生机器验收通过。原生 Windows 安装、对话及卸载验收仍待执行。

其他验证

• Codex 路径与安装单测：26 项通过。
• Hook 生命周期测试：20 项通过。
• Memory runtime 测试文件：8/21 通过；13 个扫描用例失败，表现为预期的 detect 未调用。尚未完成基线对照，因此不将该测试文件标记为全通过。
• macOS 隔离 DMG 的非默认 Codex 路径验收此前通过。

[Memtensor-AI]

🤖 Open Code Review

Target: PR #769
Task: ddc0d914c83d4d9c
Base: v1.2.7
Head: fix-codex-hook-path-v127-pr

🔍 OpenCodeReview found 13 issue(s) in this PR.

⚠️ Review warnings (2)

• bounded analysis finalized with explicit task_done after reviewing all group files
• Filtered 2 low-confidence OCR finding(s) before posting/fix-loop.

---

1. App/backend/src/adapters/outbound/skill-writer/codex/hook-trust.ts (L161)

Behavior change in CLI selection: the previous local resolver preferred the desktop-bundled CLI (plugins/.plugin-appserver, then the ChatGPT.app bundle) and only fell back to a bare codex PATH lookup. The shared resolver now searches PATH before the macOS app-bundle locations (see Memory/AgentSourceCore/src/codex-executable.ts, plugin → PATH → /Applications), and also honors MEMMY_CODEX_EXECUTABLE/CODEX_CLI_PATH. Hook trust depends on the selected binary understanding hooks/list and config/batchWrite, so a stale/standalone codex earlier on PATH can now be picked over the desktop-bundled CLI in exactly the case where the plugin app-server binary is missing. Please confirm this is intentional (or prefer the app-bundle binary before PATH) and cover it in the resolver tests.

---

2. App/backend/src/adapters/outbound/skill-writer/codex/hook-trust.ts (L54)

prepareCodexHookTrust returns a TrustMemmyCodexHooks, whose options still declare codexExecutable, but the wrapper always overwrites it. A caller passing codexExecutable (e.g. a test or custom install flow) would have it silently ignored. Either document that the resolved executable is pinned, or forward the caller value as an override:

return (options) => trustMemmyCodexHooks({ ...options, codexExecutable: options.codexExecutable ?? codexExecutable });

---

3. Memory/src/agent-source/integration/codex/hook-trust.ts (L161)

Behavior change in CLI selection: the previous local resolver preferred the desktop-bundled CLI (plugins/.plugin-appserver, then the ChatGPT.app bundle) and only fell back to a bare codex PATH lookup. The shared resolver now searches PATH before the macOS app-bundle locations (plugin → PATH → /Applications), so a stale/standalone codex earlier on PATH can now be selected instead of the desktop-bundled CLI when the plugin app-server binary is missing. Since hook trust requires the CLI to support hooks/list and config/batchWrite, please confirm this ordering is intentional and cover it in tests.

---

4. Memory/src/agent-source/integration/codex/hook-trust.ts (L54)

prepareCodexHookTrust returns a TrustMemmyCodexHooks, whose options still declare codexExecutable, but the wrapper always overwrites it. A caller passing codexExecutable would have it silently ignored. Either document that the executable is resolved/pinned up-front, or forward the caller value as an override:

return (options) => trustMemmyCodexHooks({ ...options, codexExecutable: options.codexExecutable ?? codexExecutable });

---

5. App/backend/src/adapters/outbound/skill-writer/codex/target.ts (L127)

The rollback snapshot list hardcodes the skill directory names as string literals, while the code that actually writes them (replaceMemmySkillDirectory / replaceMemmyResumeSkillDirectory) resolves the same paths through MEMMY_SKILL_DIRECTORY_NAME / MEMMY_RESUME_SKILL_DIRECTORY_NAME from templates/memmy-skill-directory.ts. If those constants ever change, the snapshot silently targets a non-existent path and a failed install will neither restore nor clean up the real skill directories (data loss of the previous contents), with no error raised. Import and use the shared constants instead (they are already available from the module that provides renderMemmySkillBootstrapManifest).

💡 Suggested Change

Before:

          join(root, "skills", "memmy-memory"), join(root, "skills", "memmy-resume"),

After:

          join(root, "skills", MEMMY_SKILL_DIRECTORY_NAME), join(root, "skills", MEMMY_RESUME_SKILL_DIRECTORY_NAME),

---

6. App/backend/src/adapters/outbound/skill-writer/codex/target.ts (L416)

Two problems with restoring directory symlinks this way:

1. On Windows, symlink(target, path, "junction") resolves a relative target against process.cwd() and converts it to an absolute junction target, so a previously relative directory link is silently repointed to an unrelated location (or fails). Only use junction when snapshot.target is absolute.
2. snapshot.directory is computed with stat (which follows the link), so a dangling directory symlink is recorded as directory: false and later recreated as a file-type link, which Windows will not resolve as a directory once the target reappears.

Also consider restoring snapshot.children with Promise.all instead of the sequential for ... await loop below, since the children are independent paths.

💡 Suggested Change

Before:

    await symlink(snapshot.target, snapshot.path, snapshot.directory ? "junction" : "file");

After:

    await symlink(snapshot.target, snapshot.path, snapshot.directory
      ? (isAbsolute(snapshot.target) ? "junction" : "dir")
      : "file");

---

7. Memory/src/agent-source/integration/codex/target.ts (L113)

The rollback snapshot list hardcodes the skill directory names as string literals, while the code that actually writes them (replaceMemmySkillDirectory / replaceMemmyResumeSkillDirectory) resolves the same paths through MEMMY_SKILL_DIRECTORY_NAME / MEMMY_RESUME_SKILL_DIRECTORY_NAME from templates/memmy-skill-directory.ts. If those constants ever change, the snapshot silently targets a non-existent path and a failed install will neither restore nor clean up the real skill directories (data loss of the previous contents), with no error raised. Import and use the shared constants instead (they are already available from the module that provides renderMemmySkillBootstrapManifest).

💡 Suggested Change

Before:

          join(root, "skills", "memmy-memory"), join(root, "skills", "memmy-resume"),

After:

          join(root, "skills", MEMMY_SKILL_DIRECTORY_NAME), join(root, "skills", MEMMY_RESUME_SKILL_DIRECTORY_NAME),

---

8. Memory/src/agent-source/integration/codex/target.ts (L398)

Two problems with restoring directory symlinks this way:

1. On Windows, symlink(target, path, "junction") resolves a relative target against process.cwd() and converts it to an absolute junction target, so a previously relative directory link is silently repointed to an unrelated location (or fails). Only use junction when snapshot.target is absolute.
2. snapshot.directory is computed with stat (which follows the link), so a dangling directory symlink is recorded as directory: false and later recreated as a file-type link, which Windows will not resolve as a directory once the target reappears.

Also consider restoring snapshot.children with Promise.all instead of the sequential for ... await loop below, since the children are independent paths.

💡 Suggested Change

Before:

    await symlink(snapshot.target, snapshot.path, snapshot.directory ? "junction" : "file");

After:

    await symlink(snapshot.target, snapshot.path, snapshot.directory
      ? (isAbsolute(snapshot.target) ? "junction" : "dir")
      : "file");

---

9. Memory/AgentSourceCore/src/codex-executable.ts (L80-L82)

For an explicit override that points at a real but non-native entry point (.cmd/.bat/.ps1 shim, or a codex.js whose sibling @openai/codex package cannot be located), fromFile returns undefined, so this branch reports "The configured Codex executable is unavailable" even though the user-supplied path exists and is spawnable. That message is factually wrong and makes such misconfiguration hard to diagnose (the user already followed the advice to pass an absolute CLI path). Distinguishing "path missing" from "entry point has no native binary" makes the failure actionable.

💡 Suggested Change

Before:

    const executable = fromFile(unquote(explicit));
    if (executable) return executable;
    throw new Error("The configured Codex executable is unavailable. Set MEMMY_CODEX_EXECUTABLE to an absolute Codex CLI path (not the desktop application).");

After:

    const configured = unquote(explicit);
    const executable = fromFile(configured);
    if (executable) return executable;
    throw new Error(paths.isAbsolute(configured) && existsSync(configured)
      ? "The configured Codex entry point has no native Codex CLI. Set MEMMY_CODEX_EXECUTABLE to the native codex binary rather than a package shim."
      : "The configured Codex executable is unavailable. Set MEMMY_CODEX_EXECUTABLE to an absolute Codex CLI path (not the desktop application).");
    // Note: import `existsSync` from "node:fs" alongside the existing imports.

---

10. package.json (L90)

test:codex-hooks duplicates the exact four-workspace build prefix already performed by test:opencode, and both are chained in test, so a plain npm test now builds @memmy/agent-source-core, @memmy/local-api-contracts, the backend workspace bridge and the Memory integration twice. Extracting the shared build prefix into one script (used by both test targets) keeps the chain correct while avoiding the redundant build pass.

💡 Suggested Change

Before:

    "test:codex-hooks": "npm run build -w @memmy/agent-source-core && npm run build -w @memmy/local-api-contracts && npm run workspace-bridge:build -w @memmy/backend && npm run integration:build -w @memmy/memory && vitest run tests/codex-hook-lifecycle.test.ts"

After:

    "build:test-deps": "npm run build -w @memmy/agent-source-core && npm run build -w @memmy/local-api-contracts && npm run workspace-bridge:build -w @memmy/backend && npm run integration:build -w @memmy/memory",
    "test:codex-hooks": "npm run build:test-deps && vitest run tests/codex-hook-lifecycle.test.ts"

---

11. App/backend/src/adapters/outbound/skill-writer/codex/target.ts (L136-L143)

These lines duplicate install() (lines 66-70) but are not identical: install() first runs removeLegacyMarkerBlock(...) on AGENTS.md, while this path does not. As a result the primary install path (installPlugin) leaves a stale <!-- memmy-memory cli : start --> block in AGENTS.md, even though uninstallPlugin removes it. Reuse install() so both paths cannot drift (it also avoids maintaining two copies of the skill-directory replacement sequence).

💡 Suggested Change

Before:

          const manifest = renderMemmyPluginSkillManifest(_targetId);
          const filePath = join(root, TARGET_FILE_NAME);
          await writeFileAtomically(
            filePath,
            upsertMarkerBlock(await readTextFile(filePath), renderMemmySkillBootstrapManifest(manifest))
          );
          await replaceMemmySkillDirectory(root, manifest);
          await replaceMemmyResumeSkillDirectory(root, CODEX_TARGET_ID);

After:

          const manifest = renderMemmyPluginSkillManifest(_targetId);
          await this.install(manifest);

---

12. Memory/src/agent-source/integration/codex/target.ts (L122-L129)

These lines duplicate install() (lines 65-69) but are not identical: install() first runs removeLegacyMarkerBlock(...) on AGENTS.md, while this path does not. As a result the primary install path (installPlugin) leaves a stale <!-- memmy-memory cli : start --> block in AGENTS.md, even though uninstallPlugin removes it. Reuse install() so both paths cannot drift (it also avoids maintaining two copies of the skill-directory replacement sequence).

💡 Suggested Change

Before:

          const manifest = renderMemmyPluginSkillManifest(_targetId);
          const filePath = join(root, TARGET_FILE_NAME);
          await writeFileAtomically(
            filePath,
            upsertMarkerBlock(await readTextFile(filePath), renderMemmySkillBootstrapManifest(manifest))
          );
          await replaceMemmySkillDirectory(root, manifest);
          await replaceMemmyResumeSkillDirectory(root, CODEX_TARGET_ID);

After:

          const manifest = renderMemmyPluginSkillManifest(_targetId);
          await this.install(manifest);

---

13. Memory/AgentSourceCore/src/codex-executable.ts (L41-L42)

These two chained ternaries are nested ternary expressions, which the project's review rules forbid. Replacing them with small lookup maps also makes the platform/arch -> vendor-triple mapping easier to extend (e.g. linux gnu/musl variants) without further nesting.

💡 Suggested Change

Before:

    const cpu = architecture === "arm64" ? "aarch64" : architecture === "x64" ? "x86_64" : undefined;
    const target = platform === "win32" ? "pc-windows-msvc" : platform === "darwin" ? "apple-darwin" : "unknown-linux-musl";

After:

    const cpu = ({ arm64: "aarch64", x64: "x86_64" } as Record<string, string | undefined>)[architecture];
    const target = ({ win32: "pc-windows-msvc", darwin: "apple-darwin" } as Record<string, string | undefined>)[platform] ?? "unknown-linux-musl";

---

🧹 Filtered 2 low-confidence OCR finding(s) before posting/fix-loop (duplicate: 2).

Generated by cloud-assistant via Open Code Review.

[Memtensor-AI]

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Executor error: Command failed with exit code 1: /opt/memos-test/.venv/bin/python -m pip install -e /data/test-workspaces/800e85c33f020865/repo
stderr: ERROR: file:///data/test-workspaces/800e85c33f020865/repo does not appear to be a Python project: neither 'setup.py' nor 'pyproject.toml' found.
Branch: fix-codex-hook-path-v127-pr

[Memtensor-AI]

⚠️ Automated Test Results: GIT PREPARATION FAILED

Git preparation did not complete. Tests were not executed; inspect the recorded command evidence.

Details: Executor error: Git clone failed (timeout, attempt 1/2, timeout 300000ms). The command deadline expired; this alone does not identify a network or authentication root cause.
Branch: fix-codex-hook-path-v127-pr