fix(storage): ignore invalid ancestor Git markers - #4807
Conversation
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
I found one correctness gap in the ancestor-marker validation. The patch fixes the reported missing-HEAD shape, but other structurally invalid ancestor markers still enter the Git path and fail project/workspace resolution. The exact-head build, Storage typecheck, full Storage suite (1,113 passed, 10 skipped), Biome, ASF header audit, diff check, and a patch-preserving merge onto current main passed. GitHub currently exposes only the label check for this head.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| const entry = await lstat(gitPath); | ||
| if (current === path) return true; | ||
| const gitStat = entry.isSymbolicLink() ? await stat(gitPath) : entry; | ||
| if (gitStat.isDirectory()) return pathExists(join(gitPath, 'HEAD')); |
There was a problem hiding this comment.
[P2] Validate the Git metadata rather than only the existence of HEAD. pathExists() returns true for any stat-able path, so an ancestor whose .git/HEAD is a directory or contains garbage, and even a repository missing objects or refs, still passes this branch. On this exact head I reproduced all four cases: hasEnclosingGitEntry() returned true, then resolveProjectLocation() exited with Git code 128 and resolveWorkspaceIdentity() returned workspace_io_failed. A gitdir: target with a garbage HEAD fails the same way. The new tests cover only the no-HEAD fixture, so the advertised invalid-ancestor behavior remains incomplete. Please validate enough of the Git directory/gitfile contract to reject these shapes and add regression coverage for at least malformed/non-regular HEAD.
Generated-by: OpenAI Codex
7b0a560 to
79730f7
Compare
hqhq1025
left a comment
hqhq1025
left a comment
There was a problem hiding this comment.
I found one remaining correctness gap in the new Git-directory validator. The newly added fixtures cover the four previously reported malformed shapes, but malformed symbolic HEAD refs can still enter the Git path and fail both project and workspace resolution. Clean installation, build:test, Storage typecheck, the full Storage suite (1,125 passed, 10 skipped), focused affected tests (42/42), changed-file Biome, ASF headers, diff check, and a clean patch-preserving merge onto current main (774389072) passed. GitHub exposes no hosted checks for this head; native Windows path and symlink behavior was not run locally.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
| // readFile fails closed on a missing, unreadable, or directory HEAD. | ||
| const head = (await readFile(join(gitDir, 'HEAD'), 'utf8')).trim(); | ||
| const validHead = head.startsWith(HEAD_REF_PREFIX) | ||
| ? head.slice(HEAD_REF_PREFIX.length).trim() !== '' |
There was a problem hiding this comment.
[P2] This accepts every non-empty ref: ... suffix, including strings that Git does not recognize as a repository HEAD. On this exact head, an ancestor containing .git/objects, .git/refs, and HEAD set to ref: gk passes isGitDirectory(), but git -C <nested> rev-parse ... exits 128. Through the production paths, resolveProjectLocation() then throws that Git failure and resolveWorkspaceIdentity() returns workspace_io_failed without publishing a marker. The malformed ancestor therefore still blocks the two workflows this PR is intended to recover. Please validate the symbolic ref shape and add this case to the shared broken-metadata fixture.
2 participants
Summary
Ignore structurally invalid
.gitentries in strict ancestors when classifying a selected folder, while preserving fail-closed behavior for a malformed.gitentry in the selected folder itself. Valid Git directories,gitdir:indirection files, repositories, and linked worktrees continue to resolve normally.Add regression coverage for Project Catalog classification and workspace identity marker publication.
Fixes #4806
Verification
npm --workspace @maka/storage run buildnpm --workspace @maka/storage run typechecknpm --workspace @maka/storage run test:dist— 1,112 passed, 8 skipped, 0 failednpx biome check packages/storage/src/git-entry.ts packages/storage/src/__tests__/project-catalog.test.ts packages/storage/src/__tests__/workspace-identity.test.tsnpm run check:asf-headerskind: 'folder'AI use
Select exactly one:
Tool(s) and scope: OpenAI Codex — root-cause analysis, implementation, regression tests, verification, and PR drafting.
Checklist
Does this PR entail a change in behavior?