add bundled starter agents and reliable first-run Home - #43
Conversation
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Static review of the exact three-dot comparison found three blocking issues and one non-blocking issue. The supplied GitHub evidence was inspected: all nine check runs for the exact head SHA completed successfully, while the legacy combined status is pending with no status contexts. Final self-check covered first-run and reset Home flows, delayed starter-agent recovery, persona loading and mutation races, bundled-agent collision handling, accessibility, localization, navigation and reset consent, async/error/never-completes behavior, lifecycle and persistence races, test honesty, design-system/project rules, duplicate overlap, and concrete user impact. No prior review threads were supplied, so no findings were suppressed as duplicates.
Deterministic publication result: 3 blocking and 1 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found five blocking and three non-blocking publishable findings. All four earlier automated threads were resolved without substantive human replies; their reported defects were verified as fixed and were not repeated. The supplied GitHub evidence was inspected: all eight check runs for the exact head SHA completed successfully, while the legacy combined status is pending with no status contexts. Final self-check covered every changed flow (first-run Home, global onboarding reset, delayed starter-pin recovery, shared persona loading/mutations, bundled-agent installation and agent behavior), accessibility, i18n, navigation and reset consent, async/error/never-completes/lifecycle/races, persistence boundaries, test honesty, design-system/project laws, duplicate overlap, and concrete user effects for blocking severity.
Deterministic publication result: 5 blocking and 3 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found five blocking and three non-blocking publishable findings. The twelve supplied prior automated threads were all resolved without substantive human replies; their fixes were checked against the full current comparison, fixed issues were not repeated, and one remaining camera-recovery defect was evaluated on its current evidence. The supplied GitHub evidence was inspected: five check runs for the exact head SHA completed successfully and Frontend checks and unit tests was in progress; the legacy combined status is pending with no status contexts. Final self-check covered every changed user flow (bundled-agent installation/repair, first-run Home, global reset, delayed starter-pin recovery, persona reads/mutations, and all new agent behaviors), accessibility, localization, navigation and consent, async/error/never-completes/lifecycle/races, persistence and filesystem trust boundaries, test honesty, design-system/project laws, duplicate overlap, and concrete user effects for blocking severity.
Deterministic publication result: 5 blocking and 3 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found three blocking and three non-blocking publishable findings. All twenty supplied prior automated threads were resolved without substantive human replies; each reported fix was checked in the current full comparison and fixed issues were not repeated. The supplied GitHub evidence was inspected: Clean-room frozen install, DCO, and macOS/Linux Rust checks passed; Windows Rust checks and Frontend checks and unit tests were in progress; the legacy combined status is pending with no status contexts. Final self-check covered every changed user flow, accessibility, localization, navigation and consent, async/error/never-completes/lifecycle/races, filesystem and persistence trust boundaries, test honesty, design-system/project laws, duplicate overlap, and concrete evidence plus user effects for blocking severity.
Deterministic publication result: 3 blocking and 3 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found four blocking and two non-blocking publishable findings. All twenty-six supplied prior automated threads were resolved without substantive human replies; each reported fix was checked against the current full comparison, fixed defects were not repeated, and incomplete ownership/recovery fixes were evaluated on their current evidence. The supplied GitHub evidence was inspected: all eight check runs for the exact head SHA completed successfully; the legacy combined status is pending with no status contexts. Final self-check covered every changed user flow (bundle seeding and repair, legacy migration, first-run Home, global reset, camera recovery, delayed starter pins, and all new agent behaviors), accessibility, localization, navigation and consent, async/error/never-completes/lifecycle/races, filesystem and persistence trust boundaries, test honesty, design-system/project laws, duplicate overlap, and concrete evidence plus user effects for blocking severity.
Deterministic publication result: 4 blocking and 2 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found one blocking and one non-blocking publishable finding. All thirty-two supplied prior automated threads were resolved without substantive human replies; their fixes were checked against the current full comparison and fixed issues were not repeated. The supplied GitHub evidence was inspected: seven check runs for the exact head SHA completed successfully and Transcript virtualization was in progress; the legacy combined status is pending with no status contexts. Final self-check covered every changed user flow (bundle seeding/repair and legacy migration, first-run Home, global reset, camera recovery, legacy and delayed starter pins, and new bundled-agent behavior), accessibility, localization, navigation and consent, async/error/never-completes/lifecycle/races, filesystem and persistence boundaries, test honesty, design-system/project laws, duplicate overlap, and concrete evidence plus user effect for blocking severity.
Deterministic publication result: 1 blocking and 1 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
* origin/main: polish home canvas widgets (#50) redesign shared and imported agent cards (#42) send queued messages in the background when the chat is not open (#41) # Conflicts: # src/features/home/onboarding/starterHomeLayout.ts # src/features/home/stores/homeWidgetRuntime.ts # src/features/home/stores/homeWidgetStore.test.ts # src/features/home/stores/homeWidgetStore.ts # src/features/home/ui/HomeView.tsx
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found four blocking and three non-blocking publishable findings. All thirty-four supplied prior automated threads were resolved without substantive human replies; their fixes were checked against the current full comparison and fixed issues were not repeated. The supplied GitHub evidence was inspected: all eight check runs for the exact head SHA completed successfully; the legacy combined status is pending with no status contexts. Final self-check covered every changed user flow (bundle validation, seeding/repair and migration, first-run Home, starter arrangement, global reset, camera recovery, legacy and delayed starter pins, experiments controls, and every bundled-agent workflow), accessibility, localization, navigation and consent, async/error/never-completes/lifecycle/races, filesystem and persistence boundaries, test honesty, design-system/project laws, duplicate overlap, and concrete evidence plus user effects for blocking severity.
Deterministic publication result: 4 blocking and 3 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found two blocking publishable findings and no non-blocking findings. All forty-one supplied prior automated threads were read with their resolution states, replies, authors, bodies, and URLs. Every thread is resolved without a substantive human reply; the current fixes were checked against the full comparison, fixed issues were not repeated, and the two incomplete fixes below remain eligible to be raised again. The supplied GitHub evidence was inspected: all eight check runs for the exact head SHA completed successfully; the legacy combined status is pending with no status contexts. Final self-check covered every changed user flow (bundle validation, installation, repair, managed-allocation identity, first-run Home, starter arrangement, full onboarding reset, camera recovery, legacy and delayed starter pins, experiments controls, and bundled-agent workflows), accessibility, localization, navigation and consent, async/error/never-completes/lifecycle/races, filesystem and persistence boundaries, test honesty, design-system/project laws, duplicate overlap, and concrete evidence plus user effects for blocking severity.
Deterministic publication result: 2 blocking and 0 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
* block/pr-43: (30 commits) fix(home): centralize reset exclusivity and arrangement confirmation fix(home): serialize starter persistence lifecycle fix(agents): select verified managed starter copies fix(agents): clarify Agt. Builder trust and consent fix(release): add bundled source identities fix(agents): recognize known legacy bundled bytes fix(home): serialize onboarding reset mutations test(home): align starter coverage with polished layout style(home): format merged starter layout fix(home): restore legacy starter pin migration fix(home): make camera recovery conditional and nonblocking fix(agents): unify bundled allocation ownership fix(agents): complete bundled manifest migration validation refactor(home): use the existing persona store for starter pins test(agents): include bundled source identity fixture fix(home): recover pending starter camera fix(agents): bound persona read generations fix(agents): harden bundled agent ownership and identity fix(agents): make bundled prompt contracts self-contained fix(onboarding): report partial reset results ...
morgmart
left a comment
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. Fresh static review of the exact three-dot comparison found three blocking and four non-blocking publishable findings. All forty-three supplied threads were read with their resolution states, replies, authors, bodies, and URLs. Two existing threads remain unresolved and were treated as authoritative duplicate memory; their underlying issues were verified as fixed at this SHA and were not repeated or added to duplicate_audit. Resolved issues were checked against the full current comparison and fixed ones were not repeated. The supplied GitHub evidence was inspected: all seven supplied check runs for the exact head SHA completed successfully; the legacy combined status is pending with no status contexts. Final self-check covered every changed user flow (bundle validation, seeding/repair, managed identity, first-run Home, deferred personas and pins, final arrangement, full reset, camera conflict recovery, legacy migration, experiment controls, and bundled-agent workflows), accessibility, localization, navigation and consent, async/error/never-completes/lifecycle/races, filesystem and persistence boundaries, test honesty, design-system/project rules, duplicate overlap, and concrete evidence plus user effects for blocking severity.
Deterministic publication result: 3 blocking and 4 non-blocking finding(s) publishable; 0 duplicate(s) suppressed.
| savedState.camera?.centerX === starterCamera.centerX && | ||
| savedState.camera.centerY === starterCamera.centerY && | ||
| savedState.camera.zoomBps === starterCamera.zoomBps; | ||
| if (cameraSaved) { |
There was a problem hiding this comment.
🤖 P1 · Base camera completes layout early (blocking)
The empty-Home initialization path marks the starter Home arranged as soon as the provisional base camera persists. markStarterHomeArranged clears starter-layout eligibility before Tinker and Wildcard can arrive asynchronously and be pinned. The reset path likewise marks the zero-pin base composition arranged after its item and camera saves. HomeView's final arrangement effect requires that eligibility, so it never applies the completed agent layout, dynamic task height, or final 80/44 at 90% camera after pins become available.
User effect: First-run and reset Home can show all starter pins eventually but remain in the interim framing and placement instead of the intended final composition.
Recommended fix: Separate base item/camera persistence from completed starter arrangement. Keep layout eligibility set until all canonical widgets, managed starter pins, final positions, task height, and final camera are confirmed; only that final operation should call markStarterHomeArranged.
Test: Start from an empty backend layout, let the base camera succeed, then deliver personas and persist pins. Assert the final arrangement and 80/44/9000 camera save occur and eligibility clears only afterward. Add the equivalent full-reset flow.
| continue; | ||
| }; | ||
| let target = target_root.join(&allocation.target_file_name); | ||
| if target.exists() && digest_file(&target)? == expected_digest { |
There was a problem hiding this comment.
🤖 P1 · Managed identity is self-attested (blocking)
verified_managed_agent_allocations treats a target as authoritative when its bytes match installedDigest from .berd-bundled-agents.json. Both the marker and agent files live in the user-writable agents directory, so a crafted marker can name any plain local agent file and provide the digest of those same bytes. The Tauri command then labels it as the managed Berdy, Tinker, or Wildcard copy, and frontend selection prefers that identity over genuine bundled candidates.
User effect: Untrusted local agent content can impersonate a Berd-managed starter and be launched or pinned from Berd-owned onboarding UI.
Recommended fix: Treat the marker only as allocation metadata. Verify the allocated file against the current packaged source bytes and validated source identity, or against another app-controlled manifest that the user-writable marker cannot mint.
Test: Create a marker that maps berdy.md to an arbitrary local prompt with its matching attacker-chosen digest. Assert the allocation command rejects it; then verify a target matching the packaged Berdy bytes is accepted.
|
|
||
| Default to building over describing. If the ask is concrete, make the thing and show it — don't narrate a plan for a thing you could just build. | ||
|
|
||
| **For the agent/skill/automation question, propose a shape before creating anything.** State what you'd build and why in a couple of lines, then create it once they confirm. This is the one place where confirming first matters more than moving fast — getting someone's first custom agent wrong is a worse outcome than a slower start. A quick tracker or script doesn't need this ceremony; build it. |
There was a problem hiding this comment.
🤖 P1 · Tinker skips side-effect consent (blocking)
Tinker's prompt says a quick tracker or script does not need confirmation and should be built immediately. Building through tools can write or overwrite files, execute commands, install dependencies, access credentials or networks, and publish output, yet this flow requires neither a bounded scope nor approval of the concrete side effects.
User effect: A conversational request for a tracker or script can trigger workspace changes or command execution before the person knows what will happen.
Recommended fix: Allow side-effect-free drafting without another pause, but require a concrete scope and explicit confirmation before writes, execution, installs, external access, credential use, or publishing.
Test: Evaluate requests for a draft-only script, a file-writing tracker, dependency installation, and a networked script. Assert only the draft proceeds immediately and every consequential operation waits for explicit approval.
| arranged, | ||
| camera ? { ...camera, centerX: 80, centerY: 44, zoomBps: 9_000 } : null, | ||
| ) | ||
| .then((confirmed) => { |
There was a problem hiding this comment.
🤖 P2 · Failed arrangement may never retry (non-blocking)
HomeView clears starterLayoutArrangementAttemptedRef only in the applyStarterLayout promise callback. Clearing a ref does not cause a render. Store updates from a save conflict or failure can render while the ref is still true; after the callback flips it, no dependency must change, leaving the eligible Home stranded until an unrelated update or remount. If batching happens differently, immediate effect retries can instead spin on a persistent failure.
User effect: A transient arrangement failure can leave Home partially arranged with no retry, while a persistent failure may cause repeated background saves and toasts.
Recommended fix: Give retries an explicit bounded state machine owned by the store/runtime: schedule a deliberate retry transition for transient failures, use backoff or a user retry affordance, and stop automatically on persistent errors.
Test: Simulate conflict then success and assert one scheduled same-mount retry completes. Simulate persistent failure and assert there is no save/toast loop and eligibility remains available for a later explicit retry.
| if (cameraSaved) { | ||
| markStarterHomeArranged(); | ||
| } else { | ||
| markStarterHomeCameraPending({ |
There was a problem hiding this comment.
🤖 P2 · Conflict recovery stores stale revision (non-blocking)
When the starter camera save loses a revision race, the camera queue adopts the conflict response's newer layout. The pending recovery record still uses cameraRevisionBeforeSave, or initialCameraRevision in reset, rather than the latest adopted revision. On next initialization, setReadyLayout sees that the pending revision differs from the backend and clears the request without retrying it.
User effect: A camera conflict during first-run or reset can leave Home incorrectly framed, and the promised recovery silently disappears on restart.
Recommended fix: When persistence fails or conflicts, record the latest confirmed camera revision from the adopted result/store, not the pre-attempt revision. Keep the target camera paired with that latest revision.
Test: Return a camera conflict at revision N, restart from backend revision N, and assert the intended starter camera retries and clears pending state only after confirmation. Cover first-run and reset paths.
| toast.error(i18n.t("home:widgetLayer.toasts.copyFailed")); | ||
| } | ||
| }, | ||
| addMissingStarterAgentPins: (agentIds, legacyBerdyAgentId) => { |
There was a problem hiding this comment.
🤖 P2 · Obsolete pin recovery clears eligibility (non-blocking)
starterAgentRecoveryPromise deduplicates all calls without considering the requested agent IDs. If managed allocation identity changes from IDs A to IDs B while A's save is pending, the B request joins A's promise. Both HomeView callbacks can mark starter pins seeded when only A persisted, removing eligibility before the current B selection is reconciled.
User effect: Home can keep dead or obsolete starter pins, or later accumulate duplicates, instead of pinning the currently verified managed agents.
Recommended fix: Key or serialize recovery by desired ID set, queue the latest request, and only report success for the currently selected IDs. Reconcile starter-owned obsolete pins when allocation IDs change.
Test: Defer recovery for IDs A, replace personas with IDs B, then resolve A. Assert the seeded marker is not written for A, B is subsequently persisted, and the final canvas contains only B.
| expect(readdirSync(agentDirectory).sort()).toEqual(starterAgentFiles); | ||
| for (const fileName of starterAgentFiles) { | ||
| const contents = readFileSync(resolve(agentDirectory, fileName), "utf8"); | ||
| expect(contents).toContain("berdBundled: true"); |
There was a problem hiding this comment.
🤖 P2 · Default bundle skips contract validation (non-blocking)
The release staging script runs validate-bundled-agents only when custom agents are selected. The default release test checks filenames, a literal berdBundled string, and one missing-reference pattern, but never invokes validateBundledAgentFile or validateBundledAgentSet over the committed distro. Therefore ordinary official builds have no unconditional release-path check for malformed frontmatter, invalid avatars, missing source IDs, or duplicate identities.
User effect: A broken starter-agent contract can pass the advertised release coverage and ship in a default release.
Recommended fix: Validate the complete distro agent set unconditionally before packaging, and have the release test call the same file and set validators rather than checking string fragments.
Test: Assert the committed seven-agent set passes full validation, then inject malformed frontmatter, a duplicate source ID, and an invalid avatar in isolated fixtures and assert the default release gate rejects each.
|
🤖 Superseded by #70, rebuilt from current main with a deliberately narrower clean-install scope and without the migration, managed-identity, camera-recovery, or alternate persistence systems added during this PR. |
2 participants
Category: new-feature
User Impact: New users start with seven useful agents, with Berdy, Tinker, and Wildcard featured on a reliable first-run Home canvas.
Problem: New users previously received only Berdy by default, while optional recommendations and Home starter pins followed separate, fragile setup paths. Resetting onboarding or loading agents slowly could also leave Home incomplete or inconsistently framed.
Solution: Bundle the complete starter-agent set and derive first-run and reset Home from one canonical layout. The flow preserves customized Homes, safely handles user-owned filename collisions, recovers delayed starter pins and avatar media, and serializes persistence before marking onboarding complete.
File changes
distro/agents/berdy.md
Updates Berdy's bundled definition and black-cube avatar reference.
distro/agents/choosey.md
Adds Choosey to the default bundled starter-agent set.
distro/agents/copycat.md
Adds Copycat to the default bundled starter-agent set.
distro/agents/pushback.md
Adds Pushback to the default bundled starter-agent set.
distro/agents/sprout.md
Adds Agt. Builder's current instructions and avatar as a bundled starter agent.
distro/agents/tinker.md
Adds Tinker to the default bundled starter-agent set and Home composition.
distro/agents/wildcard.md
Adds Wildcard to the default bundled starter-agent set and Home composition.
release-agents/README.md
Documents that starter agents are sourced from the distro bundle.
scripts/release/lib.sh
Clarifies release staging behavior for the bundled starter-agent set.
src-tauri/src/services/bundled_agents.rs
Generalizes safe fallback installation and repair for user-owned filename collisions while preserving bundled identity and updates.
src/app/AppShell.navigation.test.tsx
Updates Berdy fixtures for the new avatar identity.
src/features/agents/hooks/usePersonas.ts
Coordinates persona loads and mutations across consumers so stale requests cannot overwrite edits.
src/features/agents/hooks/tests/usePersonas.test.ts
Covers shared initial loads and cross-consumer mutation races.
src/features/experiments/ExperimentsSettings.tsx
Consolidates onboarding reset controls into one global reset action.
src/features/experiments/tests/ExperimentsSettings.test.tsx
Updates reset-control coverage for the consolidated experience.
src/features/home/onboarding/createStarterHomeWidgets.ts
Defines the canonical first-run and reset Home composition.
src/features/home/onboarding/createStarterHomeWidgets.test.ts
Covers full and partial starter layouts and stable agent slots.
src/features/home/onboarding/starterAgents.ts
Selects bundled Tinker and Wildcard by stable file identity and manages recovery markers.
src/features/home/onboarding/starterAgents.test.ts
Covers canonical/fallback identity, impostors, ordering, and marker transitions.
src/features/home/onboarding/starterHomeLayout.ts
Defines the reviewed starter positions, 100% camera framing, and smaller clock.
src/features/home/stores/homeWidgetRuntime.ts
Seeds the complete starter Home atomically and recovers gracefully when personas or camera persistence are delayed.
src/features/home/stores/homeWidgetStore.ts
Serializes starter-pin recovery and global reset, including replacement conflicts and camera-only failures.
src/features/home/stores/homeWidgetStore.test.ts
Adds regression coverage for first-run, reset, persistence, conflicts, partial recovery, camera failures, and concurrent saves.
src/features/home/ui/HomeView.tsx
Loads personas on Home and recovers missing starter-agent pins without changing customized canvases.
src/features/home/ui/HomeView.test.tsx
Covers customized-Home preservation and delayed starter-pin recovery.
src/features/home/widgets/OnboardingTourWidget.tsx
Uses Berdy's new avatar and loads its featured Home media eagerly.
src/features/home/widgets/OnboardingTourWidget.test.tsx
Covers Berdy's avatar identity, eager loading, and cache recovery after first render.
src/features/onboarding/berdyAgent.ts
Updates stable Berdy resolution for the new avatar identity.
src/features/onboarding/berdyAgent.test.ts
Updates Berdy identity coverage.
src/features/onboarding/resetOnboardingTour.ts
Makes global reset restore starter agents and mark partial resets eligible for recovery.
src/features/onboarding/resetOnboardingTour.test.ts
Covers complete and partial global reset marker behavior.
src/scripts/tests/releaseDefaults.test.ts
Verifies all seven starter definitions are included and marked bundled.