Skip to content

fix: close 2026-11 audit gaps and state-machine defects - #310

Merged
danielvm-git merged 19 commits into
mainfrom
integration/3.1.0
Nov 5, 2026
Merged

danielvm-git merged 19 commits into
mainfrom
integration/3.1.0

Conversation

@danielvm-git

@danielvm-git danielvm-git commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

User description

Closes #300 #301 #302 #303 #304 #305 #306 #307 #308 #309.

Summary

This integration PR addresses all gaps identified in the 2026-11 audit (specs/audit/2026-11-hotpaths-state-machines.md), closes all 10 open issues, delivers multi-language deploy runtime support (e81s01–s06), reconciles epic/story statuses (e46, e83, e84, e86 marked done; e90 and e81 reconciled), and maintains green tests across all Go packages and the React 19 UI suite.

Key Changes

Verification

  • go test ./... -count=1 (all 33 packages passed)
  • go test -race ./components/deploy/ ./components/cici/ ./components/monitoring/ -count=1 (0 race conditions)
  • vitest run, tsc, eslint in ui/ (95 files / 749 tests passed, zero lint/type errors)
  • bash scripts/validate-specs-yaml.sh (OK)
  • bash scripts/trace-stories.sh (OK)

PR Type

Bug fix, Enhancement, Tests


Description

  • Add atomic deploy state transitions and boot reconciliation

  • Persist CICI run deadlines and queued/running lifecycle

  • Age stale cert provisioning into failed with retry

  • Expose non-terminal state age metrics and alerts

  • Parallelize UI bootstrap and dedupe site fetches

  • Add Rust, PHP, Java, Ruby runtime defaults


Diagram Walkthrough

flowchart LR
  A["Deploy CAS transitions"] --> B["Reconcile orphaned deployments"]
  C["CICI state machine"] --> D["queued/running/deadline"]
  E["Cert provisioning budget"] --> F["failed + manual retry"]
  G["Monitoring state age"] --> H["metrics/alerts"]
  I["UI bootstrap"] --> J["parallel/dedupe/concurrency"]
  K["Runtime defaults"] --> L["Rust/PHP/Java/Ruby"]
Loading

File Walkthrough

Relevant files
Enhancement
16 files
domains.go
Persist cert attempt budget and retry endpoint                     
+256/-13
runs.go
Atomic run transitions and persisted deadline                       
+114/-14
state.go
Define CICI run state machine                                                       
+156/-0 
state_transitions.go
Compare-and-swap status transitions with retry                     
+115/-63
runtimes.go
Add multi-language build and start commands                           
+192/-0 
framework_mode.go
Support rust, java, ruby app types                                             
+1/-1     
state_age.go
Report non-terminal state age metrics                                       
+318/-0 
alert_checker.go
Trigger alerts from state age metrics                                       
+51/-10 
state_machine.go
Export canonical state predicates                                               
+58/-4   
cici.go
Add cici run migrations and reconciliation                             
+49/-5   
sites.go
Add cert attempt columns migration                                             
+28/-5   
sitesData.ts
Deduplicate getSite call in deployments                                   
+26/-3   
SiteDetailPage.tsx
Parallelize independent page bootstrap fetches                     
+8/-3     
CreateSitePage.tsx
Parallelize create site init fetches                                         
+15/-17 
boundedConcurrency.ts
Bound bulk import concurrency                                                       
+33/-0   
SiteEnvVarsTab.tsx
Use bounded pool for env var imports                                         
+14/-11 
Bug fix
3 files
reconcile.go
Reconcile orphaned in-flight deployments on boot                 
+126/-0 
mcp.go
Validate env var key format                                                           
+14/-0   
deploy.go
Wire boot reconciliation into Deploy.Start                             
+18/-2   
Tests
1 files
runtimes_test.go
Test multi-language runtime commands and profiles               
+415/-0 
Additional files
35 files
test-build-release.yml +7/-0     
.gosec.json +25/-1   
README.md +1/-1     
reconcile_test.go +145/-0 
runs_internal_test.go +79/-0   
state_test.go +108/-0 
deploy_runner.go +7/-7     
deploy_test.go +20/-0   
engine.go +39/-7   
gateway.go +2/-2     
manifest.go +12/-0   
reconcile_test.go +201/-0 
rollback.go +19/-43 
rollback_test.go +84/-0   
security_regression_test.go +32/-2   
state_machine_test.go +70/-2   
transition_test.go +79/-2   
utils.go +11/-0   
github_test.go +16/-0   
ci_template_test.go +101/-0 
ci-templates.json +27/-0   
frameworks.json +28/-0   
mcp_test.go +134/-0 
monitoring.go +7/-1     
state_age_test.go +320/-0 
domains_cert_test.go +291/-0 
sites_test.go +38/-1   
package.json +2/-2     
check-version-drift.sh +10/-0   
SiteEnvVarsTab.test.tsx +91/-0   
secretsData.test.ts +62/-0   
secretsData.ts +25/-16 
sitesData.test.ts +94/-1   
CreateSitePage.test.tsx +42/-0   
SiteDetailPage.test.tsx +57/-1   

BigBase Admin added 14 commits November 4, 2026 16:51
Deleting a site or deployment while status=deploying was allowed because both guards only checked pending/building/running. Add deploying to both sets with regression tests.
SiteDetailPage.load ran three independent GETs sequentially (every 2s during an active deploy); CreateSitePage.init ran three more. Use Promise.all and cover both with concurrency tests.
Record the 10 actionable findings from the 2026-11 audit and link issues #300-#309 from the owning epic shards.
CAS transition with retry (closes #302), remove the undeclared replaced state and document stopped as revivable (closes #303), export IsActive/IsTerminal/CanDelete and fix rollback target filtering (closes #304).
Add queued/running/terminal states with validation and persisted deadline_at (closes #305); fail orphaned queued/running rows on Start (closes #301).
Persist cert attempt budget/reason and age provisioning to failed; add POST cert retry endpoint (closes #306).
Pass the prefetched site into getSiteDeployments to remove the duplicate poll request (closes #307); run env/secret imports through a 4-wide pool preserving per-key errors (closes #309).
Mark stale pending/building/deploying rows and running rows without a port as failed through TransitionState, preserving status_history and events (closes #300).
Derive bigbase_state_age_seconds for deployment and cici_run non-terminal states, surface worst offender in metrics JSON, and evaluate state_age alert rules (closes #308).
Enforce ^[A-Z][A-Z0-9_]*$ at the MCP boundary and add integration tests for get/set/delete site env var tools (epic e83).
Single-source gosec exclusions, add UI lockfile drift guard, fix the version badge drift check, and record MCP config canonical source (e90s02/e90s04/e90s08).
Add detection, build commands, and start commands for Rust (Cargo.toml),
PHP (composer.json), Java (pom.xml/build.gradle), and Ruby (Gemfile).
Expose new runtimes in AllAppTypes(), AppType.IsValid(), manifest framework
parsing, and CI template generation in MCP. Add security regression tests.

- e81s01: Rust runtime support (cargo build --release, target/release binary)
- e81s02: PHP runtime support (composer install --no-dev, php -S)
- e81s03: Java runtime support (mvn/gradle, java -jar)
- e81s04: Ruby runtime support (bundle install, ruby app.rb/rackup)
- e81s05: App types, bigbase.toml profiles, get_ci_template for all runtimes
- e81s06: Path traversal, command injection, and sentinel regression tests
Record execution of 2026-11 audit waves closing issues #300-#309.
Reconcile epic and story statuses for e46 (done), e83 (done), e84 (done),
e86 (done), e81 (stories s01-s06 done, s00 open), and e90 (s02/s03/s05/s06/s08 done).
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

305 - Fully compliant

Compliant requirements:

  • State machine states and transitions
  • Typed transition errors
  • Persisted deadline_at and deadline enforcement
  • Legacy pending handling
  • Existing API shape

306 - PR Code Verified

Compliant requirements:

  • Persisted cert attempt timestamps and last error
  • Provisioning to failed aging with reason
  • Manual retry endpoint resets attempt budget
  • Backend test coverage for cert status/retry in domains_cert_test.go

Requires further human verification:

  • UI shows the failure reason
⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Possible Issue

TransitionState now snapshots build_log only when IsTerminal(newStatus) is true. Previously the code explicitly included StateRunning along with StateFailed. If StateRunning is a non-terminal deployment state (as the monitoring state-age spec shows), transitions to running use the status-only update path, leaving previously written build_log stale. A concurrent reader can observe status=running with an incomplete or outdated build_log, reintroducing the flake the old code was meant to fix.

// Snapshot build_log for finished states so a concurrent reader never
// observes status=running/failed with a stale build_log (the
// TestStaticSidecar_BuildAndStart flake: status and build_log were
// written in two separate statements).
var buildLog string
if d.sm.IsTerminal(newStatus) {
	buildLog = strings.Join(d.getDeployLogs(id), "\n")
}
Unhandled Errors

migrateDomains discards every s.db.Migrate(stmt) error for the ALTER TABLE statements, not just the expected duplicate-column error. If a statement fails for another reason (for example lock contention or a database dialect issue), startup continues as if migration succeeded, and later listDomains scans can fail because required columns are missing.

for _, stmt := range []string{
	`ALTER TABLE site_domains ADD COLUMN cert_attempts INTEGER NOT NULL DEFAULT 0`,
	`ALTER TABLE site_domains ADD COLUMN cert_started_at TEXT`,
	`ALTER TABLE site_domains ADD COLUMN cert_last_attempt_at TEXT`,
	`ALTER TABLE site_domains ADD COLUMN cert_last_error TEXT NOT NULL DEFAULT ''`,
	`ALTER TABLE site_domains ADD COLUMN cert_retry_after TEXT`,
} {
	_ = s.db.Migrate(stmt)
}

@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Reconcile orphaned draining deployments

The reconciliation query omits draining, even though the state machine now treats it
as in-flight. A process restart during draining leaves that deployment stuck in
draining forever, blocking site deletes and rollback. Add 'draining' to the
candidate list and treat it as stale like other in-flight statuses.

components/deploy/reconcile.go [39-43]

 rows, err := d.db.QueryContext(ctx,
     `SELECT id, status, COALESCE(created_at, ''), COALESCE(port, 0)
      FROM deployments
-     WHERE status IN ('pending', 'building', 'deploying')
+     WHERE status IN ('pending', 'building', 'deploying', 'draining')
         OR (status = 'running' AND COALESCE(port, 0) <= 0)`)
Suggestion importance[1-10]: 9

__

Why: The reconciliation query omits draining, even though the state machine now treats it as in-flight. A restart during draining can leave a deployment stuck in draining, blocking site deletion and rollback; adding it is an important correctness fix.

High
Fail closed on partial unavailability

When machine is empty, the first loop sets available = true as soon as it sees any
reading, even if other machines are in Unavailable. This causes state_age_seconds to
return (0, true) when some machines are unavailable, which can hide a stuck
deployment from alert rules. For the global metric, require all configured machines
to be available before returning ok=true; otherwise return false to fail closed.

components/monitoring/state_age.go [215-236]

 func (r StateAgeReport) maxAge(machine string) (float64, bool) {
+    if machine == "" {
+        for _, spec := range stateMachineSpecs {
+            if !r.machineAvailable(spec.Name) {
+                return 0, false
+            }
+        }
+    }
     available := false
     max := 0.0
     for _, reading := range r.ByState {
         if machine != "" && reading.Machine != machine {
             continue
         }
         available = true
         if reading.AgeSeconds > max {
             max = reading.AgeSeconds
         }
     }
     if available {
         return max, true
     }
     for _, spec := range stateMachineSpecs {
         if (machine == "" || spec.Name == machine) && r.machineAvailable(spec.Name) {
             return 0, true
         }
     }
     return 0, false
 }
Suggestion importance[1-10]: 6

__

Why: The change makes the global state_age_seconds metric fail closed when any machine is unavailable, avoiding a misleading zero. It is a reasonable robustness improvement, though more of a design decision than a critical bug fix.

Low
Fix existing table default status

This migration updates existing rows but does not change the column default.
Existing tables still default new rows to 'pending', which conflicts with the new
state machine that expects 'queued'. Because SQLite cannot alter a column default,
rebuild the table to set the new default or ensure every insert explicitly specifies
RunStatusQueued.

components/cici/cici.go [144-155]

 func (c *CICI) migrateRunColumns() error {
 	if err := c.db.Migrate(`ALTER TABLE cici_runs ADD COLUMN deadline_at TEXT`); err != nil {
 		if !isDuplicateColumnError(err) {
 			return fmt.Errorf("add cici_runs.deadline_at: %w", err)
 		}
 	}
 	if err := c.db.Migrate(
 		`UPDATE cici_runs SET status = 'queued' WHERE status = 'pending'`); err != nil {
 		return fmt.Errorf("normalize legacy pending runs: %w", err)
 	}
+	var defaultStatus string
+	if err := c.db.QueryRow(`SELECT dflt_value FROM pragma_table_info('cici_runs') WHERE name='status'`).Scan(&defaultStatus); err == nil &&
+		strings.Contains(strings.ToLower(defaultStatus), "pending") {
+		return c.db.Migrate(`
+			ALTER TABLE cici_runs RENAME TO cici_runs_old;
+			CREATE TABLE cici_runs (
+				id TEXT PRIMARY KEY, workflow_id TEXT NOT NULL, event TEXT NOT NULL,
+				status TEXT NOT NULL DEFAULT 'queued', started_at TEXT NOT NULL,
+				finished_at TEXT, deadline_at TEXT
+			);
+			INSERT INTO cici_runs (id, workflow_id, event, status, started_at, finished_at, deadline_at)
+				SELECT id, workflow_id, event, status, started_at, finished_at, deadline_at
+				FROM cici_runs_old;
+			DROP TABLE cici_runs_old;
+		`)
+	}
 	return nil
 }
Suggestion importance[1-10]: 6

__

Why: The migration updates existing rows but leaves the old default 'pending' on existing tables, so new rows may still be created with a status the new state machine treats as legacy. The rebuild approach is reasonable, though it is less critical because NormalizeRunStatus already maps pending to queued.

Low
General
Handle migration errors explicitly

Ignoring all errors from s.db.Migrate masks genuine database failures (e.g., disk
full, permission issues) that are not duplicate-column errors. This can leave the
schema partially migrated and cause runtime failures. Only ignore the
duplicate-column error (or log non-duplicate errors and return them) to ensure
migration failures are surfaced.

components/sites/domains.go [126-134]

 for _, stmt := range []string{
     `ALTER TABLE site_domains ADD COLUMN cert_attempts INTEGER NOT NULL DEFAULT 0`,
     `ALTER TABLE site_domains ADD COLUMN cert_started_at TEXT`,
     `ALTER TABLE site_domains ADD COLUMN cert_last_attempt_at TEXT`,
     `ALTER TABLE site_domains ADD COLUMN cert_last_error TEXT NOT NULL DEFAULT ''`,
     `ALTER TABLE site_domains ADD COLUMN cert_retry_after TEXT`,
 } {
-    _ = s.db.Migrate(stmt)
+    if err := s.db.Migrate(stmt); err != nil {
+        if isDuplicateColumnError(err) {
+            continue
+        }
+        return fmt.Errorf("migrate site_domains cert columns: %w", err)
+    }
 }
Suggestion importance[1-10]: 6

__

Why: Checking for non-duplicate-column errors before ignoring them prevents partial schema migration on real database failures. It is a valid error-handling improvement, but the current pattern is intentional, so impact is moderate.

Low

@danielvm-git
danielvm-git merged commit 4680163 into main Nov 5, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

P0: reconcile in-flight deployments on boot (pending/building/deploying, running with port=0)

1 participant