Skip to content

fix: inconsistient behavior with pull and redeploy button via project table#1953

Merged
kmendell merged 1 commit intomainfrom
fix/redeploy-dropdown
Mar 6, 2026
Merged

fix: inconsistient behavior with pull and redeploy button via project table#1953
kmendell merged 1 commit intomainfrom
fix/redeploy-dropdown

Conversation

@kmendell
Copy link
Copy Markdown
Member

@kmendell kmendell commented Mar 5, 2026

What This PR Implements

Related issue

Related Issue

Fixes # #1794

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

Checklist

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

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

Greptile Summary

This PR fixes the inconsistent loading/disabled-button behavior in the project table by replacing the shared isLoading state object (which applied a single spinner and disabled state across all rows when any one action was running) with a per-project actionStatus record keyed by project ID. Action logic has been extracted into a new createProjectActions factory (projects-table.actions.ts) and a shared type file (projects-table.helpers.ts), keeping the Svelte component focused on presentation.

Key changes:

  • projects-table.svelte: Swaps let isLoading = $state({...}) for let actionStatus = $state<Record<string, ActionStatus>>({}), so spinners and disabled states are now scoped to the specific row being acted on. Bulk loading state is unchanged.
  • projects-table.actions.ts: New file housing performProjectAction, handleDestroyProject, handleSyncFromGit, and the three bulk-action handlers. The bulk-action runBulkAction now correctly uses a try/finally block, ensuring isBulkLoading is always cleared — an improvement over the original code.
  • Issue: In performProjectAction (line 86) and handleSyncFromGit (line 152), handleApiResultWithCallbacks is an async function but is called without await. This makes the surrounding try/catch dead code and leaves async errors unguarded.
  • cli/go.mod: Promotes github.com/mattn/go-runewidth from indirect to direct — appears unrelated to the frontend change.

Confidence Score: 3/5

  • The core fix is correct and well-structured, but the missing await on handleApiResultWithCallbacks in two places creates dead error-handling code that should be addressed before merging.
  • The per-project loading state refactor correctly solves the reported bug, and the bulk-action finally cleanup is an improvement. The missing await on the async utility in performProjectAction and handleSyncFromGit means the catch blocks are unreachable — errors from those async flows would not surface the intended error toast, and the intended actionStatus reset in the catch would never fire. In practice handleApiResultWithCallbacks guards its own errors internally, so there is no observable regression today, but the pattern is fragile and misleading.
  • frontend/src/routes/(app)/projects/projects-table.actions.ts — the two missing await calls on handleApiResultWithCallbacks.

Important Files Changed

Filename Overview
frontend/src/routes/(app)/projects/projects-table.actions.ts New file extracting all project action logic; contains two instances where handleApiResultWithCallbacks (an async function) is called without await, making the surrounding try/catch unreachable dead code and risking unhandled promise rejections.
frontend/src/routes/(app)/projects/projects-table.helpers.ts Simple new file exporting the ActionStatus union type; correct and safe.
frontend/src/routes/(app)/projects/projects-table.svelte Refactored to use per-project actionStatus keyed by project ID instead of a shared global loading state, correctly fixing the inconsistent spinner/disabled-button behavior across rows; delegates action logic to the new createProjectActions factory.
cli/go.mod Promotes github.com/mattn/go-runewidth from an indirect to a direct dependency; unrelated to the frontend fix (discussed in a previous review thread).

Sequence Diagram

sequenceDiagram
    participant User
    participant Svelte as projects-table.svelte
    participant Actions as createProjectActions
    participant API as projectService / gitOpsSyncService
    participant UI as Toast / Dialog

    User->>Svelte: Click action button (e.g. Redeploy)
    Svelte->>Actions: performProjectAction('redeploy', id)
    Actions->>Actions: actionStatus[id] = 'redeploying'
    Actions->>API: tryCatch(config.run(id))
    API-->>Actions: Result<T, Error>
    Actions->>Actions: handleApiResultWithCallbacks(result) [NOT awaited]
    Actions-->>Svelte: returns (loading still shown via actionStatus)
    Note over Actions: async continues in background
    Actions->>UI: setLoadingState(true) → actionStatus[id] = 'redeploying'
    alt Success
        Actions->>UI: toast.success(...)
        Actions->>Svelte: refreshProjects()
    else Error
        Actions->>UI: toast.error(message)
    end
    Actions->>Actions: finally: setLoadingState(false) → actionStatus[id] = ''
    Svelte->>Svelte: isAnyLoading re-derived → buttons re-enabled

    User->>Svelte: Click Bulk Up (multiple projects)
    Svelte->>Actions: handleBulkUp(ids)
    Actions->>UI: openConfirmDialog(...)
    User->>UI: Confirm
    UI->>Actions: action callback
    Actions->>Actions: isBulkLoading.up = true
    Actions->>API: Promise.allSettled(ids.map deployProject)
    API-->>Actions: results[]
    Actions->>UI: toast (success / partial / error)
    Actions->>Svelte: refreshProjects()
    Actions->>Svelte: setSelectedIds([])
    Actions->>Actions: finally: isBulkLoading.up = false
Loading

Fix All in Codex

Last reviewed commit: 6afcff5

Greptile also left 2 inline comments on this PR.

Context used:

  • Rule from dashboard - JavaScript/TypeScript Best Practices

Use const/let instead of var for variable declarations
Prefer ... (source)

@kmendell kmendell marked this pull request as ready for review March 5, 2026 19:53
@kmendell kmendell requested a review from a team March 5, 2026 19:53
Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 5, 2026

🔍 Deadcode Analysis

Found 3 unreachable functions in the backend.

View details
pkg/libarcane/edge/transport.go:59:6: unreachable func: GetActiveTunnelTransport
pkg/utils/stdcopy/stdcopy.go:56:21: unreachable func: stdWriter.Write
pkg/utils/stdcopy/stdcopy.go:91:6: unreachable func: NewStdWriter

Only remove deadcode that you know is 100% no longer used.

Analysis from commit 5f6c17c

@getarcaneappbot
Copy link
Copy Markdown
Contributor

getarcaneappbot commented Mar 5, 2026

Container images for this PR have been built successfully!

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

Built from commit 30e6580

Comment thread frontend/src/routes/(app)/projects/projects-table.actions.ts Outdated
Comment thread frontend/src/routes/(app)/projects/projects-table.actions.ts Outdated
Comment thread cli/go.mod
@kmendell kmendell force-pushed the fix/redeploy-dropdown branch from 053d83d to 5763c13 Compare March 5, 2026 22:06
@kmendell
Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

@greptileai

Comment thread frontend/src/routes/(app)/projects/projects-table.actions.ts Outdated
@kmendell kmendell force-pushed the fix/redeploy-dropdown branch from 5763c13 to 6afcff5 Compare March 5, 2026 23:52
@kmendell
Copy link
Copy Markdown
Member Author

kmendell commented Mar 5, 2026

@greptileai

Comment thread frontend/src/routes/(app)/projects/projects-table.actions.ts
Comment thread frontend/src/routes/(app)/projects/projects-table.actions.ts Outdated
@kmendell kmendell force-pushed the fix/redeploy-dropdown branch from 6afcff5 to 7a29200 Compare March 6, 2026 00:39
@kmendell kmendell force-pushed the fix/redeploy-dropdown branch from 7a29200 to 30e6580 Compare March 6, 2026 00:41
@kmendell kmendell merged commit 0ad837c into main Mar 6, 2026
15 checks passed
@kmendell kmendell deleted the fix/redeploy-dropdown branch March 6, 2026 01:42
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