Skip to content

fix(extractor): skip hidden-dir filter for root path when using relative "."#66

Merged
spencercjh merged 4 commits intomainfrom
fix/gin
Apr 1, 2026
Merged

fix(extractor): skip hidden-dir filter for root path when using relative "."#66
spencercjh merged 4 commits intomainfrom
fix/gin

Conversation

@spencercjh
Copy link
Copy Markdown
Owner

Summary

  • When users run spec-forge generate . from a project root, filepath.Walk(".") visits the root directory "." first. The hidden-directory filter strings.HasPrefix(info.Name(), ".") matched ".", causing filepath.SkipDir to skip the entire directory tree — producing an empty openapi.yaml with 0 parsed files
  • Harden the same guard in gin/detector.go, grpcprotoc/detector.go, and gozero/detector.go so no walk function can accidentally skip the tree when called with a relative "." path
  • Convert grpcprotoc and gozero detector tests from external (_test) to internal package, merge walk regression tests into detector_test.go

Root Cause

filepath.Walk(".") → root entry has info.Name() == "."HasPrefix(".", ".") == trueSkipDir on entire tree.

Test plan

  • TestASTParser_ParseFiles_RelativeDotPath — reproduces the original bug, verifies files are parsed with "." path
  • TestASTParser_ParseFiles_HiddenDirsSkipped — confirms .hidden dirs still properly skipped
  • TestDetector_findMainFiles_RelativeDotPath — regression for gin detector
  • TestDetector_findProtoFiles_RelativeDotPath — regression for grpcprotoc detector
  • TestDetector_findAPIFiles_RelativeDotPath — regression for gozero detector
  • Full make test passes with zero failures
  • Verified against real project: 26 files parsed, 23 routes, 8 schemas extracted

When users run `spec-forge generate .` from a project root, filepath.Walk
visits "." first. The existing hidden-directory check matched "." via
HasPrefix(info.Name(), "."), causing SkipDir on the entire tree and
producing an empty openapi.yaml.

Exclude the root walk entry from the hidden-directory filter so that
relative paths work the same as absolute paths.

Signed-off-by: spencercjh <spencercjh@gmail.com>
Apply the same root-path guard from 92ad9f6 to gin/detector.go,
gozero/detector.go, and grpcprotoc/detector.go so that no walk
function can accidentally skip the entire tree when called with a
relative "." path. Add regression tests for each.

Signed-off-by: spencercjh <spencercjh@gmail.com>
Convert grpcprotoc and gozero detector tests from external (_test)
to internal package so unexported walk functions are accessible.
Fold the separate detector_walk_test.go files into detector_test.go
and remove the now-redundant files.

Signed-off-by: spencercjh <spencercjh@gmail.com>
Copilot AI review requested due to automatic review settings April 1, 2026 08:26
@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Fix hidden-dir filter skipping tree with relative "." path

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Fix hidden-directory filter skipping entire tree with relative "." path
  - Root entry "." was incorrectly matched by HasPrefix check, causing filepath.SkipDir on all files
  - Added path comparison guard to exclude root from hidden-directory filter
• Apply fix to all detector walk functions (gin, gozero, grpcprotoc)
• Add regression tests for relative "." path in all detectors
• Convert gozero and grpcprotoc tests from external to internal package
Diagram
flowchart LR
  A["filepath.Walk with relative dot path"] -->|"info.Name() == dot"| B["Hidden-dir filter check"]
  B -->|"Before: HasPrefix matches dot"| C["SkipDir entire tree - BUG"]
  B -->|"After: path != projectPath guard"| D["Skip only actual hidden dirs"]
  D --> E["Files parsed correctly"]
Loading

Grey Divider

File Changes

1. internal/extractor/gin/ast_parser.go 🐞 Bug fix +3/-2

Guard hidden-dir filter from skipping root

• Added guard condition path != p.projectPath before hidden-directory check
• Prevents root entry "." from being skipped when using relative paths
• Updated comment to clarify root exclusion logic

internal/extractor/gin/ast_parser.go


2. internal/extractor/gin/ast_parser_test.go 🧪 Tests +67/-0

Add regression tests for relative path parsing

• Added TestASTParser_ParseFiles_RelativeDotPath regression test
• Added TestASTParser_ParseFiles_HiddenDirsSkipped to verify hidden dirs still skipped
• Tests verify files are parsed with relative "." path and routes extracted

internal/extractor/gin/ast_parser_test.go


3. internal/extractor/gin/detector.go 🐞 Bug fix +3/-2

Guard hidden-dir filter in findMainFiles

• Added guard condition path != projectPath before hidden-directory check in findMainFiles
• Prevents root entry from being skipped with relative paths
• Updated comment to clarify root exclusion

internal/extractor/gin/detector.go


View more (5)
4. internal/extractor/gin/detector_test.go 🧪 Tests +23/-0

Add regression test for relative path detection

• Added TestDetector_findMainFiles_RelativeDotPath regression test
• Tests that main files are found when using relative "." path
• Verifies at least one file is discovered

internal/extractor/gin/detector_test.go


5. internal/extractor/gozero/detector.go 🐞 Bug fix +2/-2

Guard hidden-dir filter in findAPIFiles

• Added guard condition path != projectPath before hidden-directory check in findAPIFiles
• Prevents root entry from being skipped with relative paths
• Updated comment to clarify root exclusion logic

internal/extractor/gozero/detector.go


6. internal/extractor/gozero/detector_test.go 🧪 Tests +51/-30

Convert to internal package and add regression tests

• Converted test package from external gozero_test to internal gozero
• Updated all imports to use internal package functions directly
• Added TestDetector_findAPIFiles_RelativeDotPath regression test
• Merged walk regression tests into main detector test file

internal/extractor/gozero/detector_test.go


7. internal/extractor/grpcprotoc/detector.go 🐞 Bug fix +2/-2

Guard hidden-dir filter in findProtoFiles

• Added guard condition path != projectPath before hidden-directory check in findProtoFiles
• Prevents root entry from being skipped with relative paths
• Updated comment to clarify root exclusion logic

internal/extractor/grpcprotoc/detector.go


8. internal/extractor/grpcprotoc/detector_test.go 🧪 Tests +53/-33

Convert to internal package and add regression tests

• Converted test package from external grpcprotoc_test to internal grpcprotoc
• Updated all imports to use internal package functions directly
• Added TestDetector_findProtoFiles_RelativeDotPath regression test
• Merged walk regression tests into main detector test file

internal/extractor/grpcprotoc/detector_test.go


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review Bot commented Apr 1, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Ignored chdir/write errors🐞 Bug ☼ Reliability
Description
New regression tests ignore errors from os.Getwd/os.Chdir/os.WriteFile, and also ignore errors when
restoring the working directory via deferred os.Chdir. If any of these operations fails, the test
process can remain in the temp directory and cause cascading, misleading failures (including
temp-dir cleanup problems).
Code

internal/extractor/gin/ast_parser_test.go[R174-186]

+	os.WriteFile(filepath.Join(dir, "main.go"), []byte(code), 0o644)
+
+	// Use "." relative path — this is how users invoke: spec-forge generate .
+	// Regression test: info.Name() returns "." for the root, which must not be
+	// treated as a hidden directory and skip the entire tree.
+	parser := NewASTParser(".")
+
+	// Change working directory to temp dir so "." resolves correctly
+	origDir, _ := os.Getwd()
+	defer os.Chdir(origDir)
+	if err := os.Chdir(dir); err != nil {
+		t.Fatalf("failed to chdir: %v", err)
+	}
Evidence
Multiple newly added tests discard errors from filesystem operations and from restoring the working
directory, which can leave the process in the wrong CWD and make later assertions/cleanup fail for
non-obvious reasons.

internal/extractor/gin/ast_parser_test.go[162-187]
internal/extractor/gin/detector_test.go[112-133]
internal/extractor/grpcprotoc/detector_test.go[411-432]
internal/extractor/gozero/detector_test.go[362-383]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Several new tests call `os.Getwd()`, `os.Chdir()`, and `os.WriteFile()` without checking returned errors, and they also restore the working directory via `defer os.Chdir(origDir)` without checking that restoration succeeded. This can leave the test process in the temp directory and cause cascading, misleading failures (including temp-dir cleanup issues).
### Issue Context
These tests intentionally `chdir` to make `"."` resolve to a temp project root, but they should handle failures symmetrically (both entering and restoring CWD) and should fail fast with the real root cause when setup steps fail.
### Fix Focus Areas
- internal/extractor/gin/ast_parser_test.go[162-187]
- internal/extractor/gin/detector_test.go[112-133]
- internal/extractor/grpcprotoc/detector_test.go[411-432]
- internal/extractor/gozero/detector_test.go[362-383]
### Suggested changes
- Replace `origDir, _ := os.Getwd()` with error-checked `origDir, err := os.Getwd(); if err != nil { t.Fatalf(...) }`.
- Use `t.Cleanup(func() { if err := os.Chdir(origDir); err != nil { t.Fatalf(...) } })` (or at least check the error in the deferred restore).
- Check `os.WriteFile(...)`/`os.MkdirAll(...)` errors in these new tests so failures report the real cause.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@spencercjh spencercjh self-assigned this Apr 1, 2026
@spencercjh spencercjh added bug Something isn't working go Pull requests that update go code labels Apr 1, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a regression where using a relative "." project path caused directory walkers to treat the root entry "." as a hidden directory and skip the entire tree, resulting in zero parsed/collected files during extraction.

Changes:

  • Prevent hidden-directory skipping from applying to the walk root entry when projectPath is "." (and similar root cases) in gin/grpcprotoc/gozero walkers.
  • Add/extend regression tests covering relative "." path walking for gin, grpcprotoc, and gozero.
  • Convert grpcprotoc and gozero detector tests from external (*_test) to internal package tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/extractor/grpcprotoc/detector.go Adjust hidden-dir filter to avoid skipping the walk root when invoked with ".".
internal/extractor/grpcprotoc/detector_test.go Switch to internal package tests and add a relative "." regression test for proto discovery.
internal/extractor/gozero/detector.go Adjust hidden-dir filter to avoid skipping the walk root when invoked with ".".
internal/extractor/gozero/detector_test.go Switch to internal package tests and add a relative "." regression test for API discovery.
internal/extractor/gin/detector.go Adjust vendor/hidden-dir filter to avoid skipping the walk root when invoked with ".".
internal/extractor/gin/detector_test.go Add a relative "." regression test for main file discovery.
internal/extractor/gin/ast_parser.go Adjust vendor/hidden-dir filter to avoid skipping the walk root when invoked with ".".
internal/extractor/gin/ast_parser_test.go Add regression tests for parsing with relative "." and for hidden-dir skipping behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/extractor/grpcprotoc/detector_test.go Outdated
Comment thread internal/extractor/gozero/detector_test.go Outdated
Comment thread internal/extractor/gin/ast_parser_test.go Outdated
Address review feedback on PR #66: check all error return values
in the new relative-path regression tests so failures report the
real root cause instead of producing misleading results.

Signed-off-by: spencercjh <spencercjh@gmail.com>
@spencercjh spencercjh merged commit b900064 into main Apr 1, 2026
4 checks passed
@spencercjh spencercjh deleted the fix/gin branch April 1, 2026 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working go Pull requests that update go code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants