Skip to content

Fix onboarding wizard UX and post-wizard issues#269

Open
ptone wants to merge 9 commits into
mainfrom
scion/onboarding-improvements-lead
Open

Fix onboarding wizard UX and post-wizard issues#269
ptone wants to merge 9 commits into
mainfrom
scion/onboarding-improvements-lead

Conversation

@ptone

@ptone ptone commented Jun 16, 2026

Copy link
Copy Markdown
Owner

Summary

  • Runtime priority: Apple container now auto-detected ahead of podman on macOS
  • Runtime labels: "Container (generic)" replaced with "Container (Apple Virtualization)"; undetected runtimes shown as disabled with "(not detected)" hint
  • Registry input: Wizard images step now has an in-wizard input field (default: ghcr.io/homebrew-scion) instead of requiring a CLI command
  • Dir browser: New folders are auto-selected after creation; added inline typeahead filter for faster path navigation
  • Template bootstrap: Local storage fallback always initialized so default templates register in Hub DB
  • Linked folders: .scion/ directory structure now initialized when linking a local folder as a project

Test plan

  • Run scion server start on fresh install — verify wizard opens with correct runtime detection priority
  • Verify "Container (Apple Virtualization)" label on macOS, Docker/Podman disabled if not installed
  • Enter registry in wizard images step, verify images can be pulled
  • Create new folder in dir browser — verify auto-navigation into it
  • Type in filter box to filter directory entries
  • After wizard completes, verify default template appears in Hub Resources and agent creation
  • Link a local directory, verify .scion/ structure is created

ptone and others added 2 commits June 15, 2026 21:04
* chore: add implementation task briefs and finalized design docs (phases 0-6)

* feat: add Workstation flag and workstation-only middleware to ServerConfig

* feat: make dev identity configurable, default to OS user (W2)

* chore: relabel dev token as "developer token" in UI and docs (W3)

* feat: add /system/* API endpoints for workstation onboarding (W1/W2)

Add workstation-only system endpoints gated by requireWorkstation:

- GET /system/check: GatherDiagnostics returns structured results (git,
  runtime, config checks) with a ready flag
- GET /system/runtime: detect available runtime, return configured profile
- PUT /system/runtime: validate and persist runtime choice
- GET /system/status: ComputeOnboardingStatus for first-run wizard
- POST /system/init: call InitMachine with user-selected harnesses
- PUT /system/identity: already wired in Phase 1

All endpoints require loopback origin and return JSON.

* feat: add /onboarding wizard shell with steps 0-3 and done step (W1)

* feat: auto-open browser and print onboarding URL on server start (D8)

* feat: add Go-native harness image pull with per-image SSE progress (W4)

* feat: add local image build option via POST /system/images/build (W4)

* feat: wire wizard images step to pull/build endpoints (W4)

* feat: add path-safety helpers for workstation filesystem operations (W5)

* feat: add fenced fs/list, fs/mkdir, fs/validate-path endpoints (W5)

* feat: add linked-grove creation mode with directory browser to project-create (W5)

* feat: wire wizard workspace step to linked-grove and hub-native create flows (W5)

* test: add tests for workstation onboarding API and security fencing

Adds comprehensive tests for the workstation onboarding endpoints and
security primitives introduced in phases 0-5:

- requireWorkstation middleware: 404 when disabled, pass-through when enabled
- assertLoopback: table-driven IPv4/IPv6 loopback validation
- ClassifyPath: managed path detection, legacy groves path, AlreadyLinked via store
- POST /system/init: valid harnesses, unknown harness rejection, empty list rejection
- PUT /system/identity: writes and echoes display name and email
- POST /system/fs/validate-path: managed-path overlap error, normal path classification
- GET /system/fs/list: home directory listing, hidden file filtering, outside-home rejection

Also adds missing server.auth.display_name, server.auth.email, and
server.auth.username key handling in UpdateVersionedSetting, which the
identity endpoint depends on.

* docs: update README with workstation quick start and brew install

Replace the "not yet able to provide pre-built binaries" note with a
Homebrew quick start path. Lead with `brew install scion` + `scion
server start` which opens the onboarding wizard, then keep the existing
go install path as "Install from Source". Add a tip noting that the
wizard handles machine init automatically.

* chore: add round-1 review fix brief

* fix: correct home-directory boundary check in fs/list and fs/mkdir (M1)

* fix: write runtime to active profile runtime field not active_profile key (M2)

* fix: document intentional unfenced validate-path + assertLoopback (M3)

* fix: address minor review findings (m1-m5)

* chore: add round-2 review fix brief

* fix: use server-lifetime context in image pull/build goroutines (m4)

* fix: remove devUser self-shadow in auth.go (m5) and fix empty-ActiveProfile GET/PUT inconsistency (N1)

* chore: add ci-full fix task brief

* chore: add PR #264 feedback fix brief

* fix: handle top-level pull error events to prevent infinite spinner (H1)

* fix: normalize Windows path separators and drive letter breadcrumbs in dir-browser (H2, H3, M1)

* fix: check scanner.Err after build log scan loop (M2)

* fix: check context cancellation in image pull loop (M3)

* fix: add concurrency guard for concurrent image build requests (M4)

* fix: apply gofmt formatting to all Go source files

Fixes fmt-check failures across multiple packages including
telegram plugin, agent-viz, chat-app, hub, messages, and runtimebroker.

* fix: address golangci-lint findings in hub package

- Use type conversion instead of struct literal for identical
  systemIdentityRequest/Response types (staticcheck S1016)
- Check error return from srv.Shutdown in test cleanup (errcheck)

* fix: add SCION_PROJECT_ID to env var cleanup in tests and use ephemeral ports

SCION_PROJECT_ID was added to IsHubContext() but tests clearing hub
env vars were not updated, causing false hub context detection and
test failures in pkg/config, pkg/hubsync, and cmd packages.

Also switch telemetry pipeline tests from hardcoded ports to port 0
(OS-assigned ephemeral ports) to eliminate flaky port-conflict failures
from TCP TIME_WAIT between sequential tests.

* fix: resolve post-rebase build errors (unused storage import, productionMode→hostedMode)

* fix: suppress dev-auth warning in workstation mode (#72)

* chore: add phantom daemon fix task brief

* chore: add onboarding bugs 3-4-5 fix brief

* feat: detect port conflicts on server start to catch phantom daemons

* feat: add scion server stop --force to kill phantom processes by port

* fix: only resume wizard progress if user has previously started onboarding (bug 3)

* fix: display full registry-qualified image names in wizard step 5 (bug 4)

* fix: add registry and buildAvailable to system/status; hide build option for brew installs (bug 5)

* fix: store imageRegistry as component state; use it in initial and queued image name display (bug 4 complete)

* fix: scope git version check to worktree-using commands only (scion start/run)

* fix: capture needsOnboarding before daemon start to prevent /onboarding URL race (bug 6)

fix: add git version check to /system/status; show as non-blocking warning in wizard step 2 (bug 7)

* fix: fail fast on empty image_registry in pull handler; show registry-missing error in wizard step 5 (bug 8)

* fix: identity persists immediately after onboarding step 0 (bug 10); fix page title and hub-native terminology (bugs 9, 11)

- seedDevUser: reads DevUserConfig instead of hardcoding defaults
- identity PUT handler: updates DB user record so name is visible without restart
- web.go dev auto-login: reads display name/email from store instead of hardcoding
- app-shell.ts: add /onboarding to PAGE_TITLES as 'Setup'
- onboarding.ts: 'Hub-native project' -> 'Hub-managed project'

* chore: add rebase task brief for workstation-improvements

* fix: resolve post-rebase build/test errors after upstream main rebase

- pkg/hub/events.go: add PublishRaw to eventBuilder so the upstream-added
  PostgresEventPublisher satisfies the EventPublisher interface (our W4
  image-pull feature added PublishRaw to the interface).
- pkg/hub/system_handlers_test.go, fs_safety_test.go: migrate from the
  removed sqlite.New/SQLiteStore API to the upstream newTestStore helper;
  use valid UUIDs and BrokerName now required by the ent-backed store.
- cmd/server_workstation_test.go: update printWorkstationQuickstart calls
  for the needsOnboarding parameter.
- web onboarding.ts: surface runtimeAvailable in the images step (fixes a
  noUnusedLocals typecheck error) with a no-runtime warning banner.

* fix: show local build without registry and report per-harness build status

The onboarding image step hid all image options when image_registry was
unset, even though local builds don't require a registry. Now the build
option is shown when buildAvailable is true regardless of registry config,
and pull is only shown when a registry is configured.

The build handler validated requested harnesses but always built all
harnesses and emitted no per-harness status events. Now it publishes
image status events for each requested harness after a successful build
so the UI marks them complete.

* fix: resolve CI failures (fmt, lint, compat-literals, tests)

- Fix gofmt issues in cmd/root.go and pkg/messages/types.go
- Fix errcheck lint error in pkg/daemon/ports.go
- Remove ineffectual assignment in cmd/root.go usesWorktrees
- Add fs_safety.go, fs_safety_test.go, system_handlers.go to
  compat-literals allowlist
- Update broadcast tests to use new BroadcastMessage endpoint
- Update set[] error message expectations to group[]

* fix: address PR GoogleCloudPlatform#434 reviewer findings (HIGH + MEDIUM)

HIGH fixes:
- Fix navigateUp to handle root-level paths (e.g. /home → /)
- Replace syscall.Kill/SIGTERM/SIGKILL with cross-platform
  os.Process methods (os.Interrupt, process.Kill)
- Use case-insensitive path comparison helpers (pathEqual,
  pathHasPrefix) for security-sensitive path prefix checks
  in fs_safety.go and system_handlers.go

MEDIUM fixes:
- Replace http.Error with writeError for consistent JSON responses
- Convert EvalSymlinks results to absolute paths before comparing
- Add imagePullActive atomic guard to prevent concurrent pulls
- Replace doneCount counter with Set to prevent double-counting
  on retry events in onboarding image pull
- Add defensive null check for wrapper.data in SSE handler

---------

Co-authored-by: Scion <agent@scion.dev>
Co-authored-by: Scion <scion@users.noreply.github.com>
Wizard improvements:
- Prioritize apple-container over podman in runtime detection
- Replace "Container (generic)" label with "Container (Apple Virtualization)"
- Show detected/not-detected status on runtime options and disable unavailable ones
- Add in-wizard registry input field (default: ghcr.io/homebrew-scion) instead of
  requiring CLI command

Dir browser improvements:
- Auto-navigate into newly created folders instead of staying in parent
- Add inline typeahead filter in breadcrumb bar for quick path navigation

Backend fixes:
- Ensure local storage fallback is always initialized so template bootstrap
  registers default templates in the Hub database
- Initialize .scion/ directory structure when linking a local folder as a project

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a comprehensive workstation onboarding wizard and supporting APIs, enabling first-run detection, cosmetic identity configuration, Go-native image pulling/building, and linked groves support with a server-side directory browser. Feedback on the changes highlights a critical path traversal vulnerability in the workspace template import path validation, an incorrect port number in the README quickstart, a command-parsing bug where scion server start incorrectly triggers git version checks, and a Windows-specific directory navigation bug when resolving drive roots.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread pkg/hub/template_bootstrap.go Outdated
// Validate the resolved path is within the project root
absRoot, _ := filepath.Abs(projectRoot)
absDir, _ := filepath.Abs(templatesDir)
if !strings.HasPrefix(absDir, absRoot) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-critical critical

Using strings.HasPrefix(absDir, absRoot) for path validation is vulnerable to a sibling directory traversal bypass. For example, if absRoot is /home/user/project and absDir is /home/user/project-backup, the prefix check will succeed even though the directory is outside the project workspace. To prevent this, ensure that absDir is either equal to absRoot or starts with absRoot followed by the path separator.

Suggested change
if !strings.HasPrefix(absDir, absRoot) {
sep := string(filepath.Separator)
if absDir != absRoot && !strings.HasPrefix(absDir, absRoot+sep) {

Comment thread README.md Outdated
scion server start
```

Your browser will open to the onboarding wizard at `http://127.0.0.1:9810/onboarding`, which walks you through machine setup, runtime detection, harness selection, and creating your first project.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The default port for the Web UI is 8080, not 9810 (which is the default port for the broker gRPC service). Pointing users to port 9810 in the Quick Start will result in a connection error. Please update the URL to use port 8080.

Suggested change
Your browser will open to the onboarding wizard at `http://127.0.0.1:9810/onboarding`, which walks you through machine setup, runtime detection, harness selection, and creating your first project.
Your browser will open to the onboarding wizard at `http://127.0.0.1:8080/onboarding`, which walks you through machine setup, runtime detection, harness selection, and creating your first project.

Comment thread cmd/root.go
Comment on lines +437 to +443
func usesWorktrees(cmd *cobra.Command) bool {
switch cmd.Name() {
case "start", "run": // agent launch
return true
}
return false
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The usesWorktrees function checks cmd.Name() to identify agent launch commands like start or run. However, scion server start also has the leaf name start, meaning it will incorrectly return true and trigger the git version check. Since server commands do not use worktrees, we should explicitly exclude commands that have server in their command path.

func usesWorktrees(cmd *cobra.Command) bool {
	if cmd.Name() == "start" || cmd.Name() == "run" {
		for c := cmd; c != nil; c = c.Parent() {
			if c.Name() == "server" {
				return false
			}
		}
		return true
	}
	return false
}

Comment on lines +224 to +229
private navigateUp(): void {
const lastSlash = this.currentPath.lastIndexOf('/');
if (lastSlash < 0) return;
const parent = lastSlash === 0 ? '/' : this.currentPath.substring(0, lastSlash);
void this.navigate(parent);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

On Windows, if currentPath is C:/Users, navigating up will resolve parent to C:. However, C: is treated as a drive-relative path rather than the absolute drive root C:/. To ensure correct navigation to the drive root on Windows, append a trailing slash if the parent path matches a drive letter pattern.

  private navigateUp(): void {
    const lastSlash = this.currentPath.lastIndexOf('/');
    if (lastSlash < 0) return;
    let parent = lastSlash === 0 ? '/' : this.currentPath.substring(0, lastSlash);
    if (/^[a-zA-Z]:$/.test(parent)) {
      parent += '/';
    }
    void this.navigate(parent);
  }

Scion Agent (onboarding-improvements-dev) added 7 commits June 17, 2026 05:25
When the PID file is missing but the server process is still alive,
`scion server stop` now probes the default server ports (8080, 9800,
9810) instead of immediately erroring. If occupied ports are found,
it shows the user what was detected and asks for confirmation before
killing the processes. In non-interactive/auto-confirm mode the kill
proceeds automatically.
The install target printed "scion --version" but the correct
command is "scion version" (subcommand, not flag).
Dir browser: Tab key now completes the current filter — navigates into
the directory if there's a single match, or extends the filter to the
common prefix if there are multiple matches. This lets users build paths
quickly by typing + Tab.

Project creation: When a path is selected via the dir browser, the
project name field auto-fills from the final path segment if it's
empty. Applied to both the onboarding wizard and project-create page.
The POST /api/v1/projects/{id}/providers endpoint (used by both the
onboarding wizard and project-create page) was missing the InitProject
call that handleProjectRegister already had. This meant .scion/ was
never created in the linked directory when using the two-step
create-project-then-add-provider flow.
Add waitForServerReady() that polls /healthz with 250ms intervals up
to a 20-second timeout. On first start the server may take several
seconds to initialize (storage, templates, migrations), so opening
the browser immediately caused connection errors. Now the browser
only opens once the health endpoint returns 200. If the timeout
expires, the URL is still printed but the browser is not opened.
Feature 1: Embed <scion-resource-import> on the harness selection step
(step 3) so users can import additional harness configurations from a
URL. After import, the new configs appear as selectable checkboxes
alongside the built-in harnesses.

Feature 2: On the image step (step 4), show images from imported
harness configs with their actual image names. Configs that include a
Dockerfile in their file manifest are labeled "Dockerfile included"
and the build-locally gate accounts for them, so even without a
system-level build script available, users can still proceed if
imported configs have Dockerfiles.
…ive root

- Security-critical: Use separator-aware path prefix check to prevent
  sibling directory traversal in template_bootstrap.go
- Fix README Quick Start URL port from 9810 to 8080
- Exclude server commands from usesWorktrees check
- Append trailing slash for bare drive letters in dir-browser navigateUp
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.

1 participant