mirror of
https://github.com/deepseek-ai/deepseek-harness.git
synced 2026-09-09 04:02:35 +00:00
Merge pull request #3361 from deepseek-harness/revert-2573-fix/windows-path-case-test-fragility
Revert "test(session): resolve one relative root on both sides of the jsonl round-trip"
This commit is contained in:
Tianyi Cui
-6
@@ -1,6 +0,0 @@
|
||||
# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each
|
||||
# side as of the last confirmed-consistent state. Both languages carry equal authority;
|
||||
# after editing either side, bring the other along and re-record with:
|
||||
# pnpm run verify-translation-pairing --write .agents/notes/implemented/testing/2026-08-14-case-insensitive-path-round-trips.md
|
||||
2026-08-14-case-insensitive-path-round-trips.md: 5ff03a4fb55fc3dc136b22850f938685ed15a44f
|
||||
2026-08-14-case-insensitive-path-round-trips.zh.md: 6ad6121653badd3f636dda822a95fa7672b22cc3
|
||||
@@ -1,37 +0,0 @@
|
||||
# Agent Note: Case-insensitive path round-trips in test expectations
|
||||
|
||||
Status: implemented
|
||||
|
||||
English | [中文](2026-08-14-case-insensitive-path-round-trips.zh.md)
|
||||
|
||||
## Problem
|
||||
|
||||
[`packages/session/session-persistence-jsonl/tests/jsonl.spec.ts`](../../../../packages/session/session-persistence-jsonl/tests/jsonl.spec.ts) proves that a relative `root` is resolved before a session is located. It handed the plugin `relative(process.cwd(), absoluteRoot)` and built its expectation from `resolve(absoluteRoot)` — two different starting points for the same directory.
|
||||
|
||||
On a case-insensitive filesystem those starting points can disagree on spelling. `path.relative()` on Windows compares case-insensitively and returns a path with the shared prefix removed, so the casing of that prefix is gone; `path.resolve()` then rebuilds it from `process.cwd()`. When the prefix `tmpdir()` and `process.cwd()` share is spelled with different casing in each, the plugin's resolved root carries the `cwd` spelling while the expectation carries the `tmpdir()` spelling, and `toEqual` compares two strings that name the same file.
|
||||
|
||||
A host reaches that state when `tmpdir()` and `process.cwd()` share a path prefix but spell it differently — for example when `TMP` is mapped into the runner work tree under one spelling while the workspace path uses another. Sharing the tree is not enough on its own: if both spell the prefix alike, the round-trip returns the same string. The case fails there and passes everywhere else, which reads as a flake rather than as a fixed disagreement between two spellings.
|
||||
|
||||
## Decision
|
||||
|
||||
The expectation resolves the same relative root the plugin receives. Both sides pass through one `resolve(cwd, relative)` call, so the case-insensitive round-trip cannot place two spellings on the two sides of the comparison.
|
||||
|
||||
This is a test-only change. The platform treats both spellings as the same file, so storage behaviour does not depend on which spelling `resolve()` produces. The string itself stays observable: hook payloads carry it as `transcript_path` and the shell contributor exports it as `DSH_SESSION_JSONL`, so a consumer that compares those strings can still see the difference. Composition fixtures such as [`apps/cli/tests/profiles/headless/tests/fixtures/cli.cordis.yml`](../../../../apps/cli/tests/profiles/headless/tests/fixtures/cli.cordis.yml) set a relative session root — but the plugin resolves whatever it receives before use, so a relative root reaches disk as one spelling rather than two.
|
||||
|
||||
The case still asserts what it names: with the plugin's `resolve(config.root)` reduced to `config.root`, so a relative root is no longer resolved, the case fails.
|
||||
|
||||
The neighbouring decision about constructing paths with the host `node:path` API lives in [cross-platform test fixtures](2026-07-22-cross-platform-test-fixtures.md); this note covers a different mechanism, the `relative()`/`resolve()` round-trip under a case-insensitive filesystem.
|
||||
|
||||
## Alternatives considered
|
||||
|
||||
**Compare the two paths case-insensitively.** This keeps the assertion green on the affected runners but accepts a real configuration disagreement as normal, and it would spread to every future path assertion rather than staying in the one case that round-trips through `relative()`.
|
||||
|
||||
**Re-register the runners so `workFolder` matches the directory casing.** That repairs the underlying inconsistency, but `.runner` also carries the agent identity, pool, and server URLs, so hand-editing it risks a registration mismatch, and the test would remain fragile for any other host whose temp directory and working directory disagree on casing.
|
||||
|
||||
**Normalize through `realpath()` in the expectation.** `realpath()` returns the on-disk casing, which is the `cwd` spelling here, so the case would pass; it also resolves symlinks, which changes what the assertion covers on hosts where the temp directory is a link.
|
||||
|
||||
## Consequences
|
||||
|
||||
The relative-root case now depends on `resolve()` alone rather than on the two spellings agreeing, so it passes on hosts whose temp directory and working directory disagree on casing. The underlying runner registration is untouched: a `workFolder` whose spelling differs from the directory on disk stays that way, so any future assertion that compares a `tmpdir()`-derived absolute path against a `cwd`-derived one will meet the same disagreement.
|
||||
|
||||
The changed case passes, and the whole `session-persistence-jsonl` suite passes at 242 cases. The mechanism was reproduced away from Windows with `path.win32`: `relative()` on two differently-cased spellings of one directory returns a prefix-free relative path, `resolve()` rebuilds it from the `cwd` spelling, and the two absolute strings differ; with both sides spelled alike the same code matches. The regression check above — removing `resolve()` from the plugin — turns the case red while the fixture root and the working directory share a drive letter. Across drives `relative()` returns an absolute path, so both spellings already agree and the check cannot go red; the fixture roots come from `tmpdir()`, so the check only goes red where that path and the working directory share a drive.
|
||||
@@ -1,37 +0,0 @@
|
||||
# Agent Note: 测试期望值里的大小写不敏感路径往返
|
||||
|
||||
Status: implemented
|
||||
|
||||
[English](2026-08-14-case-insensitive-path-round-trips.md) | 中文
|
||||
|
||||
## 问题
|
||||
|
||||
[`packages/session/session-persistence-jsonl/tests/jsonl.spec.ts`](../../../../packages/session/session-persistence-jsonl/tests/jsonl.spec.ts) 有一条用例验证「定位 session 之前会先解析相对 `root`」。它传给插件的是 `relative(process.cwd(), absoluteRoot)`，而期望值由 `resolve(absoluteRoot)` 算出——同一个目录、两个不同的起点。
|
||||
|
||||
在大小写不敏感的文件系统上，这两个起点的拼写可能不一致。Windows 的 `path.relative()` 按大小写不敏感比较，返回的是去掉公共前缀之后的相对路径，前缀的大小写信息随之丢失；随后 `path.resolve()` 用 `process.cwd()` 重新拼出前缀。当 `tmpdir()` 与 `process.cwd()` 共有的那段前缀在两者中拼写大小写不同时，插件解析出的 root 带的是 `cwd` 那种拼写，而期望值带的是 `tmpdir()` 那种拼写，于是 `toEqual` 比较的是指向同一个文件的两个字符串。
|
||||
|
||||
当 `tmpdir()` 与 `process.cwd()` 共享一段路径前缀、但两者对它的拼写不同时，主机就处在这个状态——例如把 `TMP` 以一种拼写映射进 runner 工作树、而 workspace 路径用另一种拼写。仅仅落在同一目录树内并不够：若两者的前缀拼写相同，往返会得到同一个字符串。该用例只在那里失败、别处都通过，看起来像 flake，实际是两种拼写之间一个固定的分歧。
|
||||
|
||||
## 决定
|
||||
|
||||
期望值改为解析「插件实际收到的那个相对 root」。两侧都经过同一次 `resolve(cwd, relative)`，大小写不敏感的往返就不可能把两种拼写分别放到比较的两边。
|
||||
|
||||
这是只改测试的变更。平台把两种拼写视为同一个文件，所以存储行为不依赖 `resolve()` 产出哪种拼写。字符串本身仍可被观察到：hook 载荷以 `transcript_path` 携带它，shell 贡献者以 `DSH_SESSION_JSONL` 导出它，因此比较这些字符串的消费方仍能看出差异。组合 fixture（测试前置数据）如 [`apps/cli/tests/profiles/headless/tests/fixtures/cli.cordis.yml`](../../../../apps/cli/tests/profiles/headless/tests/fixtures/cli.cordis.yml) 就设置了相对的会话 root——但插件会先解析收到的值再使用，因此相对 root 落盘时只有一种拼写而非两种。
|
||||
|
||||
该用例仍然在验证它声称的东西：把插件的 `resolve(config.root)` 降级成 `config.root`（即不再解析相对 root）后，用例转红。
|
||||
|
||||
关于「用宿主的 `node:path` API 构造路径」这一相邻决策，归属的 note 是[跨平台测试前置数据](2026-07-22-cross-platform-test-fixtures.zh.md)；本 note 讲的是另一个机制——大小写不敏感文件系统上 `relative()`/`resolve()` 的往返。
|
||||
|
||||
## 考虑过的替代方案
|
||||
|
||||
**按大小写不敏感的方式比较两个路径。** 这能让受影响的 runner 上变绿，但等于把一个真实的配置分歧当成正常状态接受；而且这种写法会扩散到之后每一条路径断言，而不是留在唯一经由 `relative()` 往返的这一条里。
|
||||
|
||||
**重新注册 runner，让 `workFolder` 与目录大小写一致。** 这修的是底层的不一致，但 `.runner` 里同时存着 runner 的注册身份、pool 与 server URL，手工编辑有造成注册失配的风险；而且只要有别的宿主机的临时目录与工作目录大小写不一致，这条用例仍然是脆的。
|
||||
|
||||
**在期望值里用 `realpath()` 归一化。** `realpath()` 返回磁盘上的真实大小写，在这里就是 `cwd` 那种拼写，用例会通过；但它同时会解析符号链接，在临时目录本身是链接的宿主机上会改变该断言覆盖的内容。
|
||||
|
||||
## 后果
|
||||
|
||||
相对 root 那条用例现在只依赖 `resolve()` 本身，不再依赖两种拼写是否一致，因此在临时目录与工作目录大小写不一致的宿主机上也能通过。runner 注册本身未被改动：注册拼写与磁盘目录名不一致的状态会保持下去，所以今后任何拿 `tmpdir()` 派生的绝对路径去和 `cwd` 派生路径比较的断言，都会遇到同一个分歧。
|
||||
|
||||
改动后的用例通过，`session-persistence-jsonl` 整套 242 条用例通过。机制在非 Windows 环境用 `path.win32` 复现过：对同一目录的两种不同大小写拼写调用 `relative()` 会得到不含前缀的相对路径，`resolve()` 用 `cwd` 那种拼写重建，两个绝对字符串因此不同；把两侧拼写改成一致后，同一段代码即匹配。上面那条回归检查——把插件的 `resolve()` 去掉——在 fixture（测试前置数据）根与工作目录同盘符时会让用例转红。跨盘符时 `relative()` 返回绝对路径，两种拼写本就相同，该检查无法转红；本文件的 fixture 根来自 `tmpdir()`，所以只有该路径与工作目录同盘时该检查才会转红。
|
||||
@@ -171,22 +171,17 @@ describe('JsonlSessionPersistence: format helpers', () => {
|
||||
|
||||
it('resolves a relative custom root before locating a session', async () => {
|
||||
const absoluteRoot = await freshRoot()
|
||||
// Resolve the same relative root the plugin receives, not the original absolute
|
||||
// path, so both sides of the comparison pass through one `resolve()` call. See
|
||||
// .agents/notes/implemented/testing/2026-08-14-case-insensitive-path-round-trips.md
|
||||
// for why an absolute root can arrive under a different casing.
|
||||
const relativeRoot = relative(process.cwd(), absoluteRoot)
|
||||
const ctx = new Context()
|
||||
await ctx.plugin(SessionStore)
|
||||
const fiber = await ctx.plugin(JsonlSessionPersistence, {
|
||||
root: relativeRoot,
|
||||
root: relative(process.cwd(), absoluteRoot),
|
||||
compression: 'none',
|
||||
writeBatchMaxDelayMs: 1,
|
||||
})
|
||||
const m = meta('relative-location', '/work')
|
||||
expect(ctx.sessionPersistence.locate(m)).toEqual({
|
||||
kind: 'jsonl',
|
||||
path: rawLogPath(resolve(relativeRoot), '/work', m.id),
|
||||
path: rawLogPath(resolve(absoluteRoot), '/work', m.id),
|
||||
})
|
||||
await fiber.dispose()
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user