-
Notifications
You must be signed in to change notification settings - Fork 13
refactor: reorganize VM configuration and workspace settings #865
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
tomsmith8
commented
Sep 29, 2025
- Move Project Info, Repository, and Infrastructure sections from VM Config to Settings page
- Add tabbed interface in Settings: General, Repository, Graph DB
- Implement proper API key UI with write-only functionality and key icon toggle
- Simplify VM Configuration page to focus only on environment, services, and files
- Remove GitSee animation and related code from stakgraph page
- Update validation logic to handle optional fields in stakgraph store
- Maintain backward compatibility with existing API endpoints
- Move Project Info, Repository, and Infrastructure sections from VM Config to Settings page - Add tabbed interface in Settings: General, Repository, Graph DB - Implement proper API key UI with write-only functionality and key icon toggle - Simplify VM Configuration page to focus only on environment, services, and files - Remove GitSee animation and related code from stakgraph page - Update validation logic to handle optional fields in stakgraph store - Maintain backward compatibility with existing API endpoints
8f10ef0
to
7a1c0df
Compare
- Move Project Info, Repository, and Infrastructure sections from VM Config to Settings page - Add tabbed interface in Settings: General, Repository, Graph DB - Implement proper API key UI with write-only functionality and key icon toggle - Simplify VM Configuration page to focus only on environment, services, and files - Remove GitSee animation and related code from stakgraph page - Update validation logic to handle optional fields in stakgraph store - Maintain backward compatibility with existing API endpoints
7a1c0df
to
2abac68
Compare
Fixes stale closure bug where swarmApiKey could be overwritten with empty string when stakgraphData reloads. Adds swarmApiKey to dependency array per React exhaustive-deps rule.
src/components/WorkspaceSettings.tsx
Outdated
disabled={isSubmitting || stakgraphLoading || | ||
(swarmUrl === stakgraphData.swarmUrl && | ||
swarmSecretAlias === stakgraphData.swarmSecretAlias && | ||
swarmApiKey === stakgraphData.swarmApiKey)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
swarmApiKey === stakgraphData.swarmApiKey)} | |
!apiKeyTouched)} |
The save button disabled logic incorrectly compares the local API key with server data that should never contain the API key.
View Details
Analysis
Save button disabled logic incorrectly compares write-only API key in WorkspaceSettings
What fails: Infrastructure save button in WorkspaceSettings.tsx:499
compares swarmApiKey === stakgraphData.swarmApiKey
but stakgraphData.swarmApiKey
is always undefined
since the API key is write-only for security
How to reproduce:
- Navigate to workspace settings → Infrastructure tab
- Load page without making any changes - save button should be disabled but appears enabled
- Enter any API key - save button remains enabled even when other fields match server data
Result: Button disabled condition swarmApiKey === stakgraphData.swarmApiKey
always evaluates to false
(comparing string vs undefined
), so button never properly disables
Expected: Save button should only be enabled when actual changes are made. API key changes should be tracked via existing apiKeyTouched
state instead of comparing against server data
Fix: Replace swarmApiKey === stakgraphData.swarmApiKey
with !apiKeyTouched
in disabled condition to properly track API key modifications without exposing sensitive data
- Moved webhook button and handler from Pool Status page to Settings Repository tab - Webhook management now logically grouped with repository configuration - Removed webhook-related imports and handler from stakgraph page - Added handleEnsureWebhooks to WorkspaceSettings component - Webhook button appears in Repository tab when repositoryUrl exists and webhookEnsured is false
src/components/WorkspaceSettings.tsx
Outdated
const payload = { | ||
name: stakgraphData.name || workspace.name, // Use workspace name as fallback | ||
description: stakgraphData.description || workspace.description || "", | ||
repositoryUrl: repositoryUrl.trim(), | ||
// Preserve existing swarm settings | ||
swarmUrl: stakgraphData.swarmUrl || "", | ||
swarmSecretAlias: stakgraphData.swarmSecretAlias || "", | ||
swarmApiKey: stakgraphData.swarmApiKey || "", | ||
// Preserve VM settings | ||
poolName: stakgraphData.poolName || "", | ||
poolCpu: stakgraphData.poolCpu || "2", | ||
poolMemory: stakgraphData.poolMemory || "4Gi", | ||
environmentVariables: stakgraphData.environmentVariables || [], | ||
services: stakgraphData.services || [], | ||
containerFiles: stakgraphData.containerFiles || {}, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API validation schema requires all fields (name, repositoryUrl, swarmUrl, swarmSecretAlias) to be non-empty, but the save functions send payloads with empty strings for optional fields, causing validation failures.
View Details
📝 Patch Details
diff --git a/src/app/api/workspaces/[slug]/stakgraph/route.ts b/src/app/api/workspaces/[slug]/stakgraph/route.ts
index 1c4c575..9f2548f 100644
--- a/src/app/api/workspaces/[slug]/stakgraph/route.ts
+++ b/src/app/api/workspaces/[slug]/stakgraph/route.ts
@@ -25,9 +25,9 @@ const encryptionService: EncryptionService = EncryptionService.getInstance();
// Validation schema for stakgraph settings
const stakgraphSettingsSchema = z.object({
name: z.string().min(1, "Name is required"),
- repositoryUrl: z.string().url("Invalid repository URL"),
- swarmUrl: z.string().url("Invalid swarm URL"),
- swarmSecretAlias: z.string().min(1, "Swarm API key is required"),
+ repositoryUrl: z.union([z.literal(""), z.string().url("Invalid repository URL")]),
+ swarmUrl: z.union([z.literal(""), z.string().url("Invalid swarm URL")]),
+ swarmSecretAlias: z.union([z.literal(""), z.string().min(1, "Swarm API key is required")]),
swarmApiKey: z.string().optional(),
poolName: z.string().min(1, "Pool name is required"),
poolCpu: z.string().optional(),
Analysis
API validation schema mismatch causing 400 errors on partial workspace configurations
What fails: stakgraphSettingsSchema in /api/workspaces/[slug]/stakgraph
rejects payloads with empty strings for repositoryUrl, swarmUrl, and swarmSecretAlias fields, but WorkspaceSettings.tsx saveRepository() and saveInfrastructure() functions send these fields as empty strings when undefined
How to reproduce:
- Navigate to workspace settings with new tabbed interface
- Try to save repository settings without configuring swarm settings
- Try to save infrastructure settings without configuring repository URL
- API returns 400 validation error: "Invalid repository URL", "Invalid swarm URL", or "Swarm API key is required"
Result: Users cannot save partial configurations. saveRepository() fails when swarmUrl="" and swarmSecretAlias="", saveInfrastructure() fails when repositoryUrl=""
Expected: Optional fields should accept empty strings and only validate when non-empty, per Zod union pattern for optional URLs
Root cause: Schema uses z.string().url() and z.string().min(1) which reject empty strings, but frontend sends || "" fallbacks for optional fields
src/stores/useStakgraphStore.ts
Outdated
if (!state.formData.repositoryUrl.trim()) { | ||
newErrors.repositoryUrl = "Repository URL is required"; | ||
} else if (!isValidUrl(state.formData.repositoryUrl.trim())) { | ||
// Repository and Swarm settings are now in Settings page |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of keeping legacy code, can't we just get rid of old data? we have just couple working workspaces on production.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need refactor this, 500+ line component is not something we should have
- Fix save button disabled logic to use apiKeyTouched state - Update API validation schema to accept empty strings for optional fields - Remove legacy validation from useStakgraphStore - Refactor WorkspaceSettings component from 529 to 92 lines by extracting tab components - Add defaultBranch field support throughout stack - Fix githubInstallationId field reference in swarm select query
8bd83a0
to
6807bd8
Compare