Skip to content

fix(storage): ignore invalid ancestor Git markers - #4807

Open
faga295 wants to merge 1 commit into
apache:mainfrom
faga295:fix/invalid-enclosing-git-marker
Open

fix(storage): ignore invalid ancestor Git markers#4807
faga295 wants to merge 1 commit into
apache:mainfrom
faga295:fix/invalid-enclosing-git-marker

Conversation

@faga295

@faga295 faga295 commented Sep 4, 2026

Copy link
Copy Markdown

Summary

Ignore structurally invalid .git entries in strict ancestors when classifying a selected folder, while preserving fail-closed behavior for a malformed .git entry 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 build
  • npm --workspace @maka/storage run typecheck
  • npm --workspace @maka/storage run test:dist — 1,112 passed, 8 skipped, 0 failed
  • npx biome check packages/storage/src/git-entry.ts packages/storage/src/__tests__/project-catalog.test.ts packages/storage/src/__tests__/workspace-identity.test.ts
  • npm run check:asf-headers
  • Verified the affected real-world folder now resolves with kind: 'folder'

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex — root-cause analysis, implementation, regression tests, verification, and PR drafting.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Generated-by: OpenAI Codex
@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 4, 2026

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Invalid ancestor .git marker prevents adding a plain folder

2 participants