fix: allow gaze crap --analyzer on non-Go projects - #254
Conversation
em-redhat
left a comment
em-redhat
left a comment
There was a problem hiding this comment.
APPROVE
Clean, well-scoped fix that defers FindModuleRoot from the Cobra closure into runCrap, placing it after the --analyzer early-return path. This matches the pattern already used by runQuality.
Strengths:
- Correct fix location — external analyzer users never need a Go module root
p.analyzeFunc == nilguard prevents interference with test stubs- Error wrapping preserved (
fmt.Errorf("finding module root: %w", err)) - Both paths have regression tests (
TestCrapWithExternalAnalyzer_BypassesFindModuleRootandTestRunCrap_GoNativePath_FindModuleRootFailure) - Spec artifacts (proposal, design, specs, tasks) present and complete
- All 5 CI checks pass
Constitution alignment: All four principles satisfied — Accuracy (external analyzer path no longer wrongly blocked), Minimal Assumptions (no Go module assumption for non-Go projects), Actionable Output (error messages reference actual problems), Testability (both paths covered).
No blocking findings.
Defer FindModuleRoot until after the --analyzer check so external analyzer users are not blocked by a missing go.mod. Closes unbound-force#250 Assisted-by: grok-4.6 Generated with AI assistance (grok-4.6)
4e3f4b3 to
c893e6a
Compare
|
🤖 Finished Review · ✅ Success · Started 10:07 AM UTC · Completed 10:24 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.94 |
|
Risk Assessment: moderate (2/5) DetailsSmall, focused bug fix (16 net code lines in main.go) with OpenSpec artifacts inflating file count. main.go has elevated fix/revert churn but the change is narrow in scope. Linked issue #250 is a clear bug report with reproduction steps, and the PR scope is proportionate. |
ReviewFindingsLow
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
fullsend-ai-review
Bot
added
the
ready-for-merge
|
🤖 Finished Retro · ✅ Success · Started 12:18 PM UTC · Completed 12:29 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $4.48 |
Retro: PR #254 — fix: allow gaze crap --analyzer on non-Go projectsTimeline
Workflow QualityReview quality: Good. The review agent found a genuine bug the human reviewer missed — the same unconditional Human review: Strong. em-redhat provided a detailed approval covering fix location, error wrapping, test coverage, spec artifacts, CI status, and constitution alignment. The human and agent reviews were complementary — the human verified correctness of the fix itself, while the agent detected the same pattern surviving in a sibling command. Rework rate: Zero. Single commit, no fix iterations, no changes-requested cycles. The PR was well-scoped and the code was right on the first attempt. Token cost: Reasonable. $6.94 for a thorough multi-dimension review with challenger adjudication on a 7-file, 282-line PR. Cache read ratio was high (1.9M cache_read vs 156K cache_create), indicating efficient context reuse. Existing Issues Corroborated
ProposalsOne proposal filed — the Proposals filed
|
runReport called loader.FindModuleRoot unconditionally at the top of the function, causing gaze report --analyzer to fail with "no go.mod found" on non-Go projects. This is the same bug pattern fixed in runCrap by PR #254 (issue #250). The fix makes FindModuleRoot conditional on p.analyzerFlag == "": when --analyzer is set, the project may not be a Go module, so cwd is used directly as moduleDir. When --analyzer is not set (Go-native path), FindModuleRoot resolves the module root as before. Added two regression tests: - TestReportWithExternalAnalyzer_BypassesFindModuleRoot: verifies that runReport with --analyzer from a non-Go directory produces an analyzer discovery error, not "no go.mod found" - TestRunReport_GoNativePath_FindModuleRootFailure: verifies that runReport without --analyzer still calls FindModuleRoot and produces the expected error format All three commands (crap, quality, report) now follow the same pattern: check --analyzer before calling FindModuleRoot. Closes #257
runReport called loader.FindModuleRoot unconditionally at the top of the function, causing gaze report --analyzer to fail with "no go.mod found" on non-Go projects. This is the same bug pattern fixed in runCrap by PR #254 (issue #250). The fix makes FindModuleRoot conditional on p.analyzerFlag == "": when --analyzer is set, the project may not be a Go module, so cwd is used directly as moduleDir. When --analyzer is not set (Go-native path), FindModuleRoot resolves the module root as before. Added two regression tests: - TestReportWithExternalAnalyzer_BypassesFindModuleRoot: verifies that runReport with --analyzer from a non-Go directory produces an analyzer discovery error, not "no go.mod found" - TestRunReport_GoNativePath_FindModuleRootFailure: verifies that runReport without --analyzer still calls FindModuleRoot and produces the expected error format All three commands (crap, quality, report) now follow the same pattern: check --analyzer before calling FindModuleRoot. Closes #257
3 participants
Summary
gaze crap --analyzerfailed immediately withError: no go.mod foundon non-Go projects becauseFindModuleRootran before the--analyzerflag was checked. External analyzer users could not use the crap command at all.This defers module-root resolution until after the
--analyzerdispatch, matching the quality command. Go-native analysis still resolvesgo.mod; external analyzers use the working directory.Closes #250.
How to Test
From a directory without
go.mod:gaze crap --analyzer nonexistent --language python .Expect an analyzer-discovery error, not
no go.mod found.Without
--analyzerin a non-Go directory, expectfinding module root: ....How to Demo
gaze crap --analyzer <binary> --language python .no go.mod foundgaze crap ./...and confirm CRAP analysis still worksKey Files Changed
cmd/gaze/main.goFindModuleRootfrom Cobra closure intorunCrapafter--analyzercheckcmd/gaze/external_analyzer_test.goopenspec/changes/crap-external-analyzer-fix/This PR was generated by /uf.finale (AI-assisted).