Skip to content

refactor: create project filesystem architecture#2525

Open
kmendell wants to merge 1 commit intomainfrom
refactor/projects-fs
Open

refactor: create project filesystem architecture#2525
kmendell wants to merge 1 commit intomainfrom
refactor/projects-fs

Conversation

@kmendell
Copy link
Copy Markdown
Member

@kmendell kmendell commented May 8, 2026

Checklist

  • This PR is not opened from my fork’s main branch

What This PR Implements

Fixes:

Changes Made

Testing Done

  • Development environment started: ./scripts/development/dev.sh start
  • Frontend verified at http://localhost:3000
  • Backend verified at http://localhost:3552
  • Manual testing completed (describe):
  • No linting errors (e.g., just lint all)
  • Backend tests pass: just test backend

AI Tool Used (if applicable)

AI Tool:
Assistance Level:
What AI helped with:
I reviewed and edited all AI-generated output:
I ran all required tests and manually verified changes:

Additional Context

Disclaimer Greptiles Reviews use AI, make sure to check over its work.

To better help train Greptile on our codebase, if the comment is useful and valid Like the comment, if its not helpful or invalid Dislike

To have Greptile Re-Review the changes, mention greptileai.

Greptile Summary

This PR introduces a ProjectFilesystem abstraction (backend/pkg/projects/filesystem.go) that centralises all project directory operations behind a staging/promotion model, replacing scattered os.Rename, os.RemoveAll, and direct WriteProjectFiles calls spread across project_service.go. It also adds filesystem rollback when a DB save fails after promotion, and fixes the 0700 staging-directory permission bug by immediately chmoding to 0755.

  • New ProjectFilesystem type with StageCreate, StageUpdate, StageGitSync, and Promote, including atomic backup-and-restore on promotion failure.
  • preparePromotedProjectRollbackInternal in project_service.go snapshots the live directory before Promote so the filesystem can be restored if the subsequent db.Save call fails — covering both UpdateProject (including rename) and ApplyGitSyncProjectFiles.
  • addWatchInternal in fswatch/watcher.go extends symlink-aware watching by also adding the resolved real target when followSymlinks is enabled, with rollback if the primary watch add fails.

Confidence Score: 5/5

Safe to merge — the refactoring is well-structured, the staging/promotion model is correct, and integration tests cover the key rollback paths for both rename and git-sync scenarios.

The core logic (staging, promotion, backup/restore) is implemented correctly and covered by new integration tests. No functional defects were found in the changed paths. Style nits around the Internal suffix convention are the only findings.

No files require special attention beyond the minor naming nits in project_service.go and filesystem_test.go.

Comments Outside Diff (1)

  1. backend/pkg/projects/filesystem.go, line 1316-1327 (link)

    P2 Orphaned hidden directories when both promotion and restore fail

    If os.Rename(stagePath, targetLivePath) fails and the subsequent restore os.Rename(backupPath, currentLivePath) also fails, the function returns immediately without cleaning up either backupPath or stagePath. Both hidden directories (.arcane-project-backup-* and the stage dir) are abandoned under the projects root. Repeated failures would accumulate orphaned directories. Consider adding a best-effort _ = os.RemoveAll(stagePath) inside the restore-failure branch before returning.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: backend/pkg/projects/filesystem.go
    Line: 1316-1327
    
    Comment:
    **Orphaned hidden directories when both promotion and restore fail**
    
    If `os.Rename(stagePath, targetLivePath)` fails and the subsequent restore `os.Rename(backupPath, currentLivePath)` also fails, the function returns immediately without cleaning up either `backupPath` or `stagePath`. Both hidden directories (`.arcane-project-backup-*` and the stage dir) are abandoned under the projects root. Repeated failures would accumulate orphaned directories. Consider adding a best-effort `_ = os.RemoveAll(stagePath)` inside the restore-failure branch before returning.
    
    How can I resolve this? If you propose a fix, please make it concise.

    Fix in Codex Fix in Claude Code

Fix All in Codex Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
backend/internal/services/project_service.go:2865
The `rollback` and `cleanup` methods on the unexported `promotedProjectRollbackInternal` type are missing the required `Internal` suffix per the team's naming convention, which requires all unexported functions to carry this suffix to distinguish private helpers from the public API.

```suggestion
func (r *promotedProjectRollbackInternal) rollbackInternal() error {
```

### Issue 2 of 3
backend/internal/services/project_service.go:2882
The `cleanup` method is also missing the required `Internal` suffix. Both call-sites (`promoteRollback.cleanup()`) would need to be updated alongside the rename.

```suggestion
func (r *promotedProjectRollbackInternal) cleanupInternal() {
```

### Issue 3 of 3
backend/pkg/projects/filesystem_test.go:189-191
The test helper `ptrString` is an unexported package-level function without the required `Internal` suffix. The naming convention applies to all unexported functions, including test helpers.

```suggestion
func ptrStringInternal(value string) *string {
	return &value
}
```

Reviews (6): Last reviewed commit: "refactor: create project filesystem arch..." | Re-trigger Greptile

Context used:

  • Rule used - What: All unexported functions must have the "Inte... (source)

@kmendell kmendell marked this pull request as ready for review May 8, 2026 04:45
@kmendell kmendell requested a review from a team May 8, 2026 04:45
Copy link
Copy Markdown
Member Author

kmendell commented May 8, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@getarcaneappbot
Copy link
Copy Markdown
Contributor

getarcaneappbot commented May 8, 2026

Container images for this PR have been built successfully!

  • Manager: ghcr.io/getarcaneapp/arcane:pr-2525
  • Agent: ghcr.io/getarcaneapp/arcane-headless:pr-2525

Built from commit 23d007b

Comment thread backend/internal/services/project_service.go
@kmendell kmendell force-pushed the refactor/projects-fs branch 4 times, most recently from eef3982 to a4438b1 Compare May 8, 2026 05:11
Comment thread backend/pkg/projects/filesystem.go
Comment thread backend/internal/services/project_service.go
@kmendell kmendell force-pushed the refactor/projects-fs branch 3 times, most recently from a1f12e0 to ddfb994 Compare May 8, 2026 17:38
@kmendell kmendell force-pushed the refactor/projects-fs branch from ddfb994 to 23d007b Compare May 9, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants