Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
165 changes: 165 additions & 0 deletions docs/architecture-audit-2026-07-30/SetupReadinessImplementation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
# Architecture Audit — Setup Readiness Implementation

**Date:** 2026-07-30
**Scope:** goal-driven setup FSM, secret-free tool detection, Cloud membership,
repo policy, history import, sync verification, destination routing, native
setup reentry accelerator
**Mode:** implementation audit

## Acceptance criteria

- [x] Goal-specific visible steps and guards replace slide-index completion.
- [x] Progress is resumable and normalized through one versioned schema.
- [x] Completion persists progress and outcome in one settings write.
- [x] Key detection cannot retain secret-bearing RPC fields.
- [x] Cloud create/join has one shared membership command boundary.
- [x] Organization success requires authoritative roster convergence.
- [x] Admin and member team paths have distinct mutation permissions.
- [x] Workspace, Project, Work Item, Session, and Org remain separate concepts.
- [x] Dismissed/completed users can reopen the same checklist.
- [x] Personal, work-management, and team goals land on existing product
surfaces.

## Layer 1 — Compilation correctness

- `npm run typecheck`: passed.
- ESLint over every changed TypeScript/TSX file: passed.
- Focused Vitest suite: 26 tests passed across flow guards, secret-safe
detection, hidden test reentry, route reentry, Settings reentry, outcome
migration, and the atomic settings commit boundary.
- Native menu changes are checked through the `system_services` crate; no
network or persistence wire schema changed.

## Layer 2 — Dead code and structural deduplication

- `useCloudOrgMembershipActions` is called by both onboarding and
`CreateCollabOrgView`; create/join auth refresh, alias creation, and roster
convergence no longer have parallel implementations.
- Existing Key validation, external-history rescan, workspace remote resolver,
appearance settings, Cloud policy clients, sync engine, tutorial registry,
Chat Panel tab factories, and sidebar scope atoms remain authoritative.
- Legacy presentation-only onboarding steps remain exported for now; removing
them is a separate cleanup because other imports/tests may still reference
them. They are no longer in the production `STEP_CONFIGS` execution path.

## Layer 3 — Naming consistency

| Name | Meaning |
| -------------------------- | -------------------------------------------------------- |
| `SetupWalkthroughProgress` | Durable, secret-free decisions and confirmed results |
| `SetupOperation` | One foreground mutation/detection command |
| `selectedOrgId` | Managed Cloud organization membership selected for setup |
| `repoScopes` | Normalized Git remote identities, never local paths |
| `verifiedAt` | The current team-path postcondition completed |

The app retains the existing `SetupWalkthrough` route/component name for
compatibility; user-facing copy calls it a setup checklist/readiness flow.

## Layer 4 — Semantic overloading

The implementation does not merge domain objects to simplify the wizard:

| Term | Product meaning in the flow |
| ------------ | ---------------------------------------------- |
| Workspace | Local folders/files used by an agent |
| Session | Agent conversation and execution context |
| Work Item | Trackable task with status/assignee/discussion |
| Project | Planning container for Work Items |
| Organization | Cloud membership and team policy boundary |

The dedicated work-model step exposes these distinctions before the user lands
in Work Management.

## Layer 5 — Default branch analysis

- No goal is selected by default; the first transition is guarded.
- Personal and work-management goals omit Cloud steps rather than silently
pretending team readiness.
- Sharing defaults to `metadata_only`, retaining an explicit privacy boundary.
- A local folder with no Git remote is rejected as a team scope.
- Tool detection failures resolve per provider and do not fail the complete
scan; the UI still reports “not found” rather than claiming success.
- Future/unknown persisted progress fails schema parsing and resets to the
version-1 initial state.

## Layer 6 — Cross-domain concept leakage

`useSetupWalkthroughController` is an application-layer orchestrator. It reads
domain state and invokes domain-owned commands; it does not implement
credential parsing, Git remote normalization, Cloud membership, policy RPCs,
sync traversal, appearance persistence, or Chat Panel tab construction.

The only setup-owned persisted data is readiness metadata. No domain credential
or remote session payload is copied into setup state.

## Layer 7 — New developer confusion test

- Step visibility is pure (`getVisibleSetupStepIds`).
- Transition guards are pure (`canCompleteSetupStep`).
- Navigation reachability is pure (`canNavigateToSetupStep`).
- Secret stripping is named and independently tested
(`sanitizeDetectedTool`).
- Foreground operations have explicit names and one active-operation gate.
- The acceptance matrix lives next to the feature in `TEST_CASES.md`.

## Layer 8 — Wire protocol and serialization

- The detection RPC remains secret-bearing, but `sanitizeDetectedTool`
constructs a new allow-listed summary synchronously; tests prove API key,
session token, and environment values do not survive.
- `createCloudInvite` retains the existing plaintext-on-device/hash-on-wire
contract.
- Repo scopes sent to `cloud_set_org_repo_scopes` come only from
`resolveShareableScopeKeys`.
- Sharing floor uses the existing typed `CollabSessionAccessMode`.
- Repo scopes and sharing floor are two existing server RPCs, not one
transaction. The UI reports success only after both finish; if the second
fails, the step stays incomplete and is safely retryable. No false
cross-RPC atomicity is claimed.
- No live payload capture was added; existing client schema tests remain the
wire-format gate.

## Layer 9 — Init parity across entry points

| Entry point | Shared path |
| ----------------------------------- | ----------------------------------------------- |
| First Release run | Setup route → readiness FSM |
| Settings “Setup checklist” | Same route and persisted FSM |
| DOM shortcut fallback | Atomic setup-only reset → same route and FSM |
| macOS/Linux native app menu | `menu-reopen-setup` → same reset and route |
| Windows custom/native menu | `menu-reopen-setup` → same reset and route |
| Existing organization selection | Same `Org2CloudOrg` roster and namespaced scope |
| Create organization from setup | Shared membership hook |
| Create organization from regular UI | Shared membership hook |
| Join via pasted link/code | Shared membership hook and parser |

OS deep-link acceptance remains owned by the existing Cloud deep-link handler;
it converges on the same roster atom, which setup reads when reopened.

## Layer 10 — Resolver symmetry

All step decisions resolve in the same order:

1. normalize persisted progress;
2. derive visible steps from goal;
3. normalize current step against the visible set;
4. enforce the current step’s postcondition;
5. persist the next progress snapshot;
6. on completion, atomically persist terminal outcome plus final progress.

Cloud membership similarly resolves auth refresh → mutation → roster
postcondition → local selection. A superseded/expired session cannot commit a
false organization result.

## Verification

- `rustfmt --edition 2021 --check src-tauri/crates/system-services/src/app_menu.rs`: passed.
- `cargo check -p system_services`: passed.

## Architecture verdict

All 10 layers were covered. The implementation removes the disconnected
slide-deck architecture and introduces one small orchestration layer over
existing domain owners. Remaining risks are explicit: Cloud admin policy spans
two server RPCs, sync-engine drain does not provide a server receipt count, and
the live team path depends on the existing cloud/dual-instance E2E suites.
79 changes: 79 additions & 0 deletions docs/architecture-audit-2026-07-31/SetupWalkthroughCleanup.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
# Architecture Audit — Setup Walkthrough Cleanup

**Scope:** readiness onboarding entry points, step registry, WizardSystem
navigation boundary, and legacy setup export chain

**Date:** 2026-07-31

**Auditor:** Codex

## Acceptance criteria

- Production setup imports only the active readiness-flow step implementation.
- Current onboarding behavior and persisted flow state remain unchanged.
- Active/completed/locked navigation state has one shared presentation owner.
- No removed legacy symbol remains reachable through a direct or barrel import.
- TypeScript, focused behavior tests, ESLint, and systematic reference sweeps
pass.

## Entry point and ownership trace

`SetupWalkthrough/index.tsx` is the production setup surface. Its visible steps
come from `STEP_CONFIGS` in `flow.ts`; every configured renderer is implemented
by `steps/ReadinessSteps.tsx`. The controller remains the authoritative owner of
current step, completed step ids, navigation guards, persistence, and terminal
outcome. `WizardStepNavigation` receives a projection of that state and owns
presentation only.

The deleted files were reachable only from the obsolete `steps/index.ts` and
`components/index.ts` re-export chains. Neither barrel was imported by the
production flow after the active `SetupOperationError` import was changed to
its concrete readiness module.

## Deleted legacy chain

| Symbol/file | Producing path | Verdict |
| ------------------------------------- | ------------------------------------------------------ | ---------------------------------- |
| `AnimatedTitle`, `AnimatedTitleProps` | `components/AnimatedTitle.tsx` → `components/index.ts` | Delete; no active caller |
| `CompleteStep` | `steps/CompleteStep.tsx` → `steps/index.ts` | Delete; absent from `STEP_CONFIGS` |
| `DevPassportStep` | `steps/DevPassportStep.tsx` → `steps/index.ts` | Delete; absent from `STEP_CONFIGS` |
| `GitHubStep` | `steps/GitHubStep.tsx` → `steps/index.ts` | Delete; absent from `STEP_CONFIGS` |
| `RepoStep` | `steps/RepoStep.tsx` → `steps/index.ts` | Delete; absent from `STEP_CONFIGS` |
| `ThemeSelectionStep` | `steps/ThemeSelectionStep.tsx` → `steps/index.ts` | Delete; absent from `STEP_CONFIGS` |
| `WelcomeStep` | `steps/WelcomeStep.tsx` → `steps/index.ts` | Delete; absent from `STEP_CONFIGS` |
| `AGENT_CODE_NAMES` | `constants.ts` | Delete; no active caller |

## Layer review

| Layer | Coverage | Result |
| -------------------------------- | -------- | ------------------------------------------------------------------------------------------- |
| 1. Compilation and imports | Covered | Direct readiness import resolves; removed barrels have no consumers. |
| 2. Structure and dead code | Covered | Obsolete step/component chains deleted; shared navigation removes duplicated structure. |
| 3. Types and naming | Covered | Removed orphan `AnimatedTitleProps`; generic navigation item/props preserve step-id typing. |
| 4. Domain ownership | Covered | Controller/flow retain setup state and guards; the primitive owns no domain state. |
| 5. State transitions | Covered | Existing `goToStep` and `canNavigateToSetupStep` paths are reused without new transitions. |
| 6. Persistence | Covered | No schema or writer changed; setup progress writes remain in the controller/settings path. |
| 7. Error/async boundaries | Covered | Busy state disables navigation; async selection remains delegated to the owner. |
| 8. Wire protocol | Skipped | No RPC, serialization, or protocol change. |
| 9. Initialization parity | Skipped | No new initialization path or default. |
| 10. Resolver/runtime integration | Skipped | No resolver, worker, or backend integration change. |

## Systematic sweeps

- Removed-symbol search covers all of `src` and returns no references.
- Production imports of `SetupWalkthrough/steps` and
`SetupWalkthrough/components` barrels return no references.
- Raw interactive elements under `src/modules/SetupWalkthrough` return no
matches; native interaction now lives at the shared component boundary.
- Arbitrary `text-[Npx]` classes in SetupWalkthrough and edited WizardSystem
primitives return no matches.

## Verification and remaining risk

Focused static-render tests cover navigation current/completed/locked/busy
states and the shared semantic description primitive. Existing flow and i18n
tests cover the unchanged state machine and localized content. TypeScript and
ESLint cover import/export integrity.

Remaining risk is limited to visual density at uncommon viewport/font-scale
combinations; no data, persistence, protocol, or backend risk was introduced.
43 changes: 43 additions & 0 deletions docs/architecture-audit-2026-08-02/OnboardingReadinessFlow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
# Architecture Audit — Onboarding Readiness Flow

**Date:** 2026-08-02
**Scope:** first-run preferences, persisted one-time handoff, sidebar guide completion, cloud invite success boundary

## Verdict

Pass. The flow has one persisted owner (`general.setupWalkthroughProgress`), uses canonical product facts where they exist, and records only the two education actions that have no durable domain equivalent. No UI shadow state or background lifecycle was introduced.

## Layer review

| Layer | Coverage | Verdict | Evidence |
| --- | --- | --- | --- |
| 1. Compile / type integrity | Full | pass | `pnpm typecheck`; typed enum-backed handoff and milestone values. |
| 2. Dead code / parallel paths | Full | pass | Removed the three presentation variants and duplicate walkthrough sidebar; advanced theme remains in canonical Settings. |
| 3. State machine / ownership | Full | pass | `idle → pending → shown`; completion transitions are idempotent and persisted through a functional Settings atom. |
| 4. Semantic overload | Full | pass | Session and organization are derived facts; invite and Team Inbox actions use explicitly named education milestones. |
| 5. Catch-all state / types | Full | pass | No boolean catch-all or string-pattern branching; schema enums constrain every new state. |
| 6. Cross-domain boundaries | Full | pass | Progress helpers live under `store/settings`; cloud invite code no longer imports onboarding UI and writes only after `createCloudInvite` succeeds. |
| 7. Naming / API surface | Full | pass | `requestSetupGuideHandoff`, `consumeSetupGuideHandoff`, and `completeSetupGuideMilestone` describe transition intent. |
| 8. Wire protocol / persistence compatibility | Full | pass | No RPC shape changed; Zod defaults normalize valid legacy progress with `idle` and `[]`. |
| 9. Initialization parity | Reviewed, not changed | pass | No new init entry point; completed setup and hidden test-entry converge through the same stored progress schema. |
| 10. Resolver symmetry | Not applicable | pass | No multi-source resolver or fallback chain was added. |

## State and edge-case matrix

| Condition | Behavior |
| --- | --- |
| Finish succeeds | Preferences completion and pending handoff persist in one batch, then Workstation opens. |
| Finish persistence fails | User remains on setup; no completed outcome or handoff is published. |
| Guide opens with pending handoff | Panel opens and persists `shown`. |
| Handoff persistence fails | Panel remains usable; stored `pending` can retry on a later mount. |
| User skipped setup | No handoff is armed. |
| Existing completed user lacks new fields | Defaults to `idle`; no surprise auto-open. |
| Invite API fails | Invite milestone remains incomplete. |
| Invite succeeds but education persistence fails | Invite remains successful; guide milestone can remain incomplete without corrupting domain truth. |
| Repeated completion action | Functional update returns the same object and avoids a disk write. |

## Verification

- 38 focused Vitest assertions across setup, locale shape, preference interaction, guide progress, and panel actions.
- TypeScript typecheck and focused ESLint pass.
- Real Tauri flow verified: centered three-row setup → Workstation → one-time four-row guide; existing session/org facts produced `2/4` as expected.
Loading
Loading