Skip to content

Keep new agent chats as drafts until the first message is sent - #103

Open
sambitcreate wants to merge 2 commits into
mainfrom
feature/draft-agent-chats
Open

Keep new agent chats as drafts until the first message is sent#103
sambitcreate wants to merge 2 commits into
mainfrom
feature/draft-agent-chats

Conversation

@sambitcreate

@sambitcreate sambitcreate commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Opening New Agent or an empty workspace previously saved a blank conversation immediately. Ordinary desktop chats now stay transient until the first user message is durably committed. Leaving an unsent draft adds no sidebar entry; first-send failures retain the draft, and navigation during saving cannot pull the user back.

The first message and chat are installed together with bounded turn admission, validated workspace/settings/attachments/skills, an idempotent private receipt, and existing storage reconciliation. Promotion keeps the same chat identity so the first response is not canceled by a route remount.

The authorized one-time startup migration snapshots readable zero-message identities and fingerprints before checking eligibility, then removes eligible workspace chats through the existing cross-store deletion path. Bot/Assistant/Telegram/scheduled conversations, unreadable records, staged artifacts, actual private execution history, and every chat containing a message are preserved. Unknown eligibility preserves and checkpoints the candidate instead of keeping a future sweep window open. Validated header-only journals, including completed empty v3-to-v4 promotions, are cleaned up. Later launches do not repeatedly sweep new remote-created chats.

Validation:

  • Five Electron acceptance tests pass for abandonment, promotion, migration/restart, failure/retry, late receipts, and unreadable-history preservation without future resweeps.
  • Focused first-message, migration, draft, sidebar/composer/transition, storage/IPC and Remote contract tests pass.
  • TypeScript, ESLint, production build, and generic iOS build-for-testing pass.
  • Full npm test and existing Electron chat/queue/attachment/workspace checks passed locally. Final draft/migration/failure tests passed, including private-history preservation and header-only cleanup.
  • Final head 44ad722950e33dcacea2c36685fde679e583ffd4 passed all applicable GitHub checks: CI verify, Deterministic Electron E2E, Release consumer contract, and Pullfrog (no new issues found). Android was skipped by the changed-path filter as expected. The review follow-up also passed 113 focused tests and five Electron acceptance tests locally.

No remote HTTP contract or native chat-creation flow changes. Physical iOS devices were offline and the local Android toolchain was unavailable, so no local device/runtime acceptance is claimed.

@pullfrog pullfrog 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.

Important

Two things need attention before this merges: the widened post-send abandon guard changes ordinary (non-draft) append behavior — a message whose save finishes after you switch chats now silently never gets a reply — and the migration's fail-closed startup boundary only covers snapshot-save failures, leaving an unrecorded-snapshot retry window that can sweep chats created in between launches (plus one corrupt Pi journal stalls the migration forever). Both have concrete fixes sketched inline.

Reviewed changes — initial review of the full PR: renderer drafts (chat-draft.ts) replacing eager chat creation across sidebar/layout/root-view/pane; atomic first-message commit (parseChatFirstMessagecreateFirstMessageCommitter → store createWithFirstMessage) with fingerprint receipt, turn admission and workspace-mutation admission; draft composer freeze and post-promotion seeding; the one-time legacy empty-chat migration with snapshot/fingerprint/DataStore receipt, cross-store eligibility guards, and the new PiCompactionSessionStore.hasChatHistory read-only inspection; plus new unit/e2e coverage and test-script registrations.

ℹ️ Nitpicks

  • Overlapping new-chat activations can orphan a draft record. Every creation site calls createChatDraft before awaiting navigation, and discard happens only in the navigation catch or on a mounted ChatPane release. A rapid second activation (double-click "New Agent", chat.new plus a sidebar click) that supersedes the first navigation leaves the first draft with zero owners, sending: false, and no code path that ever removes it for the window's lifetime; history-back to that URL then remounts a live-looking blank draft instead of the "no longer available" state. The serialized E2E clicks cannot hit it. A busy guard around draft creation (or an ownership timeout) would close it.
  • Committed first-message receipt vs. 60 s turn TTL. The retained receipt keeps its append-bytes reservation charged until turn release; if the generation-start IPC were ever delayed past the TTL, the message stays durably sent with no reply and re-sending duplicates it. This mirrors pre-existing append semantics (generation start is prompt in practice), so it is awareness only.
  • Minor copy drift: docs/plans/draft-agent-chats-plan.md and the PR body promise "Later launches do not repeatedly sweep new remote-created chats" — finding 2 inline shows the sweep window is wider than the guard enforces; the plan text is worth aligning once the boundary decision is made.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Flash𝕏

Comment thread renderer/main/chat-pane.tsx Outdated
Comment thread main/index.ts
Comment thread main/services/pi-compaction-session-store.ts

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes — incremental re-review of 44ad7229 (delta since the prior pullfrog review at c7b202d1), plus a full re-read of the 33-file diff and the three previously-raised threads:

  • Scoped the post-send abandon guard to draft promotions only — the mountedRef/chatIdRef arms now apply solely when firstDraft, restoring the pre-PR detached-reply contract for ordinary appends, with source-level tests pinning both guard shapes.
  • Made the migration snapshot phase fully fail-closed: enumeration, candidate reads, and the receipt save are wrapped in EmptyChatMigrationSnapshotError, so startup stops before writers are admitted whenever the immutable candidate boundary cannot be persisted; eligibility-store reads are deferred until after the snapshot is saved.
  • Added per-candidate preservation (readCandidate + onPreserved) so unreadable payloads or eligibility errors preserve just that candidate and checkpoint it out — the migration still completes for healthy siblings, one corrupt Pi journal no longer stalls it forever, and no later launch re-enumerates newer chats.
  • Narrowed the pre-delete fence to the frozen fingerprint + zero-message check so the cross-store removal never reopens already-deleted private stores.
  • Added isCompletedEmptyMigration to hasChatHistory, recognizing a completed empty v3→v4 promotion (receipt, .v3-backup, indexed scaffolding) as disposable via digest and exact-record validation against the real migration output, while every deviation preserves — covered by new pi-compaction-core tests and the e2e promoted-empty scenario.
  • Made createChatDraft prune unowned, non-sending drafts superseded by a newer activation, closing the orphan-draft window, with a lifecycle test.
  • Extended the e2e migration spec with the unreadable-private-history preservation case and updated the plan doc and .papercuts.

I ran the focused suites for the changed areas in this environment: pi-compaction-core.test.ts (54/54), empty-chat-migration.test.ts (10/10), and chat-draft + chat-transition (30/30) all pass, and an independent data-loss audit of the migration and disposable-scaffold classifier found no reachable path that deletes real private data — every uncertainty direction preserves.

Pullfrog  | View workflow run | Using DeepSeek Flash𝕏

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