Fix hidden file deletion and improve file tree state management#70
Closed
josh-padnick wants to merge 2 commits intomainfrom
Closed
Fix hidden file deletion and improve file tree state management#70josh-padnick wants to merge 2 commits intomainfrom
josh-padnick wants to merge 2 commits intomainfrom
Conversation
…files) After clicking "Delete Files" in the generated files alert, the file tree panel was not cleared, leaving stale entries (including hidden files/folders like .github) visible in the UI even though they were deleted on disk. Changes: - Clear the GeneratedFiles context (setFileTree(null)) after successful deletion so the UI reflects the actual filesystem state - Fix stale React state bug: deleteFiles() now returns a boolean so handleDeleteFiles can reliably check success instead of reading a stale closure-captured deleteError value - Add backend tests confirming deleteDirectoryContents and countFilesInDirectory correctly handle hidden files/folders https://claude.ai/code/session_01Js4Mce3EEoW3Z83kpNuSEC
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
WalkthroughAdds hidden-file-aware backend tests and expands frontend flow to expose rendering control: deleteFiles now returns a boolean; GeneratedFiles context gains renderingEnabled/setRenderingEnabled; App and template rendering respect that and clear the UI file tree after successful deletions. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant App as App UI\n(GeneratedFilesContext)
participant API as GeneratedFiles API
participant FS as Filesystem
rect rgba(63, 81, 181, 0.5)
User->>App: Click "Check generated files"
App->>API: GET /generated-files/check
API->>FS: Count files (include hidden)
FS-->>API: file count
API-->>App: count response
App-->>App: set renderingEnabled (true if allowed)
end
rect rgba(0, 150, 136, 0.5)
User->>App: Click "Delete generated files"
App->>API: POST /generated-files/delete
API->>FS: delete files (including hidden)
FS-->>API: deletion result
API-->>App: { success: true, deletedCount }
App-->>App: if success -> setFileTree(null), setRenderingEnabled(false)
App-->>User: show success + deletedCount
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/generated_files_test.go`:
- Around line 711-713: The test currently ignores json.Unmarshal errors and
fails to assert HTTP status in the end-to-end delete test: update each decode
block that reads into checkResp (and the other similar response vars) to check
the error returned by json.Unmarshal and call t.Fatalf or t.Fatalf-like
assertion on error, and before decoding assert the HTTP response status (e.g.,
checkRec.Code or the http response object) equals the expected status (200 or
404 as appropriate); locate the occurrences using the variables checkRec,
checkResp and the second "check" request sequence and add assertions for status
before calling json.Unmarshal and handle/unpack json.Unmarshal errors instead of
discarding them.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
api/generated_files_test.goweb/src/App.tsxweb/src/components/layout/GeneratedFilesAlert/GeneratedFilesAlert.tsxweb/src/hooks/useApiGeneratedFilesDelete.ts
Comment on lines
+711
to
+713
| var checkResp GeneratedFilesCheckResponse | ||
| json.Unmarshal(checkRec.Body.Bytes(), &checkResp) | ||
|
|
There was a problem hiding this comment.
Assert decode/status errors in the end-to-end delete test
Line 712, Line 731, and Line 759 ignore json.Unmarshal errors, and the second check request path (Line 754-Line 760) doesn’t assert HTTP status before decode. That can let malformed responses pass quietly.
🛠️ Suggested patch
- var checkResp GeneratedFilesCheckResponse
- json.Unmarshal(checkRec.Body.Bytes(), &checkResp)
+ var checkResp GeneratedFilesCheckResponse
+ if err := json.Unmarshal(checkRec.Body.Bytes(), &checkResp); err != nil {
+ t.Fatalf("Failed to decode check response: %v. Body: %s", err, checkRec.Body.String())
+ }
- var deleteResp GeneratedFilesDeleteResponse
- json.Unmarshal(deleteRec.Body.Bytes(), &deleteResp)
+ var deleteResp GeneratedFilesDeleteResponse
+ if err := json.Unmarshal(deleteRec.Body.Bytes(), &deleteResp); err != nil {
+ t.Fatalf("Failed to decode delete response: %v. Body: %s", err, deleteRec.Body.String())
+ }
// Verify check endpoint now shows no files
checkReq2, _ := http.NewRequest("GET", "/api/generated-files/check", nil)
checkRec2 := httptest.NewRecorder()
router.ServeHTTP(checkRec2, checkReq2)
+ if checkRec2.Code != http.StatusOK {
+ t.Fatalf("Second check endpoint returned %d: %s", checkRec2.Code, checkRec2.Body.String())
+ }
var checkResp2 GeneratedFilesCheckResponse
- json.Unmarshal(checkRec2.Body.Bytes(), &checkResp2)
+ if err := json.Unmarshal(checkRec2.Body.Bytes(), &checkResp2); err != nil {
+ t.Fatalf("Failed to decode second check response: %v. Body: %s", err, checkRec2.Body.String())
+ }Also applies to: 730-732, 754-760
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/generated_files_test.go` around lines 711 - 713, The test currently
ignores json.Unmarshal errors and fails to assert HTTP status in the end-to-end
delete test: update each decode block that reads into checkResp (and the other
similar response vars) to check the error returned by json.Unmarshal and call
t.Fatalf or t.Fatalf-like assertion on error, and before decoding assert the
HTTP response status (e.g., checkRec.Code or the http response object) equals
the expected status (200 or 404 as appropriate); locate the occurrences using
the variables checkRec, checkResp and the second "check" request sequence and
add assertions for status before calling json.Unmarshal and handle/unpack
json.Unmarshal errors instead of discarding them.
…ert resolves
TemplateInline components with generateFile={true} auto-execute on page mount,
which was regenerating files (like .github/workflows/ci.yml) immediately after
the user deleted them. The user would delete files, refresh, and see them
reappear because:
1. Generated files check runs first → finds 0 files → no alert
2. TemplateInline auto-renders → recreates the files
Fix: add a `renderingEnabled` flag to GeneratedFilesContext that starts false
and is only set to true once the generated files check completes and either:
- No existing files were found, or
- The user resolved the alert (clicked "Keep Files" or "Delete Files")
TemplateInline now checks this flag before auto-rendering when generateFile
is true, preventing the race condition where files are regenerated before the
user decides what to do with existing ones.
https://claude.ai/code/session_01Js4Mce3EEoW3Z83kpNuSEC
4 tasks
Collaborator
Author
|
Closing in favor of #79, which extracts the two legitimate bug fixes from this PR:
The other changes in this PR were determined to be unnecessary after analysis:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes the handling of hidden files and directories (those starting with ".") in the generated files deletion flow, and improves the file tree state management to ensure the UI properly reflects deleted files.
Key Changes
Test Coverage: Added three comprehensive tests to verify hidden file handling:
TestDeleteDirectoryContents_HiddenFiles: Verifies thatdeleteDirectoryContentsremoves both hidden and non-hidden files/directoriesTestCountFilesInDirectory_HiddenFiles: Ensures file counting includes hidden files and files within hidden directoriesTestHandleGeneratedFilesDelete_HiddenFiles: End-to-end test of the HTTP handler flow with hidden directories like.githubDelete Hook Return Type: Changed
useApiGeneratedFilesDeletehook to return a boolean indicating success/failure:deleteFiles()now returnsPromise<boolean>instead ofPromise<void>trueon successful deletion,falseon errorFile Tree State Management: Updated
App.tsxto properly clear the file tree after deletion:setFileTreeto theuseFileTreehook destructuringsetFileTree(null)inhandleFilesDeletedto clear stale generated files from the UIAlert Component Logic: Simplified
GeneratedFilesAlertto use the boolean return value:successreturn value fromdeleteFiles()instead of checking for errorsImplementation Details
The changes ensure that hidden files and directories (like
.github,.env,.generated) are properly handled throughout the deletion flow, from the backend file system operations through to the frontend UI state management.https://claude.ai/code/session_01Js4Mce3EEoW3Z83kpNuSEC
Summary by CodeRabbit
New Features
Bug Fixes
Tests