feat: Refactor agent and hub sync for better readability

[maximbigler]

Type of change

• 🐛 Bug fix
• 🚀 New feature
• ❓ Other (please specify): Refactor

Description

Fixes #172
Fixes #211

Refactors the repository-sync and application-deploy flow in internal/hub/applications to make it readable and to remove duplicated/divergent logic. No new feature; behavior is preserved except for the intentional changes called out below.

Application-level deploy state machine

• Introduced updateApplicationStatus as the single chokepoint for every application status write + SSE publish; the four mark* helpers are now thin wrappers over it. Fixes a bug where markDeploymentInProgress published an SSE event twice on a DB error.
• Extracted applyDeployResult (+ deployNotifications) — one shared interpreter of an agent's deploy outcome (err / nil result / !Success / success), replacing two divergent copies that previously lived in the sync path and the manual-deploy path.
• Split Deployer.StartDeploy into dispatchDeploy (transport-only) and StartDeploy (dispatch + mark in-progress). The sync path uses dispatchDeploy via DeployAndWait because processSyncJob already owns the in-progress transition, so each path now marks the application in-progress exactly once instead of twice.
• Rewrote processSyncJob to use linear control flow instead of the success bool + defer pattern.
• Bug fix: terminal status writes now use context.Background() instead of the job context. A deploy that hit the 3-minute job timeout previously wrote its failure status on an already-cancelled context, which could leave the application stuck displaying Syncing.

Unified sync entry points

• Added SyncApplications, the single path that all four sync triggers funnel through (polling, manual repo sync, webhooks, GitHub Actions). It performs consistent repository-status bookkeeping (Syncing → Success/Failed).
• Added CommitResolver with StaticCommit (webhooks and GitHub Actions already carry the pushed SHA) and LatestCommit (polling and commit-less generic webhooks), separating "where the commit comes from" from the shared sync logic.
• SyncRepository, WebhookHandler, handleGenericWebhook, and the GitHub Actions handler now delegate to SyncApplications; the bespoke enqueueGenericApps helper and the inline repository-status updates were removed.

Documentation

• Doc comments make the two-layer model explicit (repository-level sync vs. application-level deploy) and clarify that a repository's SyncStatus reflects whether the sync was dispatched (commits resolved, jobs enqueued), not whether every application finished deploying — per-application progress lives on each Application.

Additional context

Intentional behavior changes (please review)

1. A sync-path transport failure (agent dropped / nil result) now marks the application OutOfSync only, instead of also Unhealthy — matching the manual-deploy path. A dropped connection does not mean the running containers are unhealthy.
2. Webhooks and GitHub Actions now perform the full Syncing → Success/Failed repository bookkeeping like the poller. Previously webhooks marked Success only (never Failed) and GitHub Actions did no repository-status bookkeeping at all. For these synchronous handlers the Syncing state is effectively instantaneous; it is only visibly observable on the polling path during the network commit lookup.
3. Generic webhooks now skip applications with an empty branch (consistent with the poller) and mark the repository Failed if commit resolution fails, instead of silently enqueuing an empty commit.

Out of scope (planned follow-up)

Removing the Default* package-level globals in favor of dependency injection, which also requires reordering the server.go bootstrap. Deliberately left for a separate PR to keep this diff focused and reviewable.

Testing

applications, routes, and websocket suites pass; gofmt / go vet clean. The only failing tests are the Docker-daemon-dependent ones in internal/agent/docker, which this branch does not touch.

[codecov]

Codecov Report

❌ Patch coverage is 69.35484% with 133 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
backend/internal/hub/routes/applications.go	57.14%	14 Missing and 7 partials ⚠️
backend/internal/agent/websocket.go	73.77%	12 Missing and 4 partials ⚠️
backend/internal/hub/websocket/handler.go	54.54%	13 Missing and 2 partials ⚠️
backend/internal/hub/db/name_hash.go	0.00%	14 Missing ⚠️
backend/internal/agent/docker/deploy.go	53.84%	7 Missing and 5 partials ⚠️
backend/internal/hub/websocket/deploy.go	73.33%	7 Missing and 5 partials ⚠️
backend/internal/hub/applications/sync.go	73.17%	7 Missing and 4 partials ⚠️
backend/internal/hub/routes/webhooks.go	52.94%	4 Missing and 4 partials ⚠️
backend/internal/hub/applications/repo_sync.go	74.07%	6 Missing and 1 partial ⚠️
backend/cmd/hub/key_rotate.go	0.00%	4 Missing and 1 partial ⚠️
... and 5 more

📢 Thoughts on this report? Let us know!

[alex289]

Manual deploy on an offline agent now returns 500 (was 409), and the test no longer covers it

DeployApplicationHandler (routes/applications.go:442) calls fire-and-forget TriggerApplicationDeploy, which does return an error when hub.Send fails — so the handler hits c.JSON(http.StatusInternalServerError, …). The old code returned 409 Conflict ("agent is not connected"), which is the correct semantic and is exactly what the new DeleteApplicationHandler still does (ErrAgentOffline → 409). So delete and deploy are now inconsistent for the same offline condition.

Worse, TestDeployApplicationHandler_AgentUnavailable was changed to use a bare &stubRouteDeployer{} (no startErr), so it returns nil and the test asserts 202 with a comment claiming "Agent-not-connected handling is still a TODO." The test no longer exercises the offline path at all — its name is now misleading. Either restore a stub that simulates the offline error and assert 409, or rename the test to reflect what it checks.

[alex289]

Delete timeout mismatch: hub 30s vs. agent 5min → state divergence

agentDeleteTimeout = 30s (hub) but the agent's executeDelete uses deploymentTimeout = 5 * time.Minute for compose down. If teardown takes 30s–5min, the hub times out (504), keeps its record, and tells the user it failed — but the agent goes on to complete the removal (containers + deployment dir gone). The hub now references an application the agent has destroyed: the inverse of the "don't lose track of running containers" goal this flow is designed around. Consider aligning the hub wait with the agent's execution timeout, or making the agent abort teardown when its delete context is cancelled.