Skip to content

fix: scheduled-task provider resolution, journalless Pi rollout generation, remote 4xx evidence - #105

Merged
sambitcreate merged 5 commits into
mainfrom
fix/scheduled-provider-and-pi-rollout
Sep 11, 2026
Merged

fix: scheduled-task provider resolution, journalless Pi rollout generation, remote 4xx evidence#105
sambitcreate merged 5 commits into
mainfrom
fix/scheduled-provider-and-pi-rollout

Conversation

@sambitcreate

Copy link
Copy Markdown
Owner

Summary

Three production failures diagnosed from an installed v0.39.0 machine's logs and userData drove this branch. Plan: docs/plans/scheduled-provider-and-pi-rollout-recovery-plan.md (index updated).

Scheduled tasks: "Choose a provider before running this scheduled task."

UI-created tasks never stored a provider, and the main-process fallback settings.lastProviderId was a dead key (only the Telegram flow wrote it; the app's real selection lives in renderer localStorage).

  • Attended chat:start calls now seed lastProviderId/lastModel via a rememberSelection seam in chat-generation-start.ts (attended-renderer-only handler; scheduled/subagent/bot streams bypass it).
  • New task drafts prefill a usable pinned provider; the editor gains an App-default / explicit Provider + Model picker following the existing Select patterns, plus a no-provider guardrail note.
  • schedule_task rejects providerless LLM creation when no app default resolves, and approval summaries render "App default (Name)" / "No provider set" instead of the misleading "Scheduler default".

Pi rollout: "Pi v4 journal creation is outside the active device rollout stage."

Generation opened the v4 journal unconditionally and hard-failed without one, so every chat created before the rollout build's first launch (and any chat with a deferred v3 migration, and the rollback env) was blocked from all generation.

  • PiCompactionSessionStore.openChatIfEligible() probe: structured reasons instead of throws; openChat keeps its exact fail-closed contract for background callers (existing tests unchanged).
  • Ineligible generation runs journalless over createInMemoryPiSession (child-agent precedent): VCC recall omitted, effect recovery never marked durable/acknowledged, durable-store quarantine never fires for in-memory failures, todo replay runs against the empty in-memory journal.
  • chats:todoSnapshot returns the unavailable snapshot without minting journals; compaction resolves already_compact benignly.
  • The fail-closed rollout contract survives verbatim: no v4 journal created, no v3 migration, compaction/memory stay cohort-gated.

Remote 4xx diagnosis evidence

remote-request-failed/remote-request-slow journal events now carry content-free method (enum) and route (strict bounded grammar over server-emitted route templates — never query strings, credentials, or raw caller paths).

Testing

  • npm run type-check, full npm run lint
  • test:scheduled 107, test:assistant-automations 132, test:todo 80, test:compaction 272, test:diagnostics 67, test:aiden-remote 357+10+7 — all green
  • npm run test full chain green; test:subagents green
  • New files registered: main/handlers/chats.test.ts (test:todo), renderer/components/scheduled-task-editor.test.tsx (test:scheduled)

Notes for reviewers

  • Known pre-existing on main (not this branch): test:google-provider has 3 failures against the chore: refresh models.dev catalog commit (verified with this diff stashed).
  • Follow-up operator step (desktop rollout durability on real devices): the runbook in docs/testing/pi-compaction-phase7-rollout-gates.md (evaluation + installed receipts, then npm run pi-upgrade:advance -- migrated_low_risk_chats).
  • Shared server contracts: wire protocol untouched; journal field additions are desktop-diagnostics-only, so no iOS/Android consumer changes are required.

…ation, remote 4xx evidence

- Seed settings.lastProviderId/lastModel from attended chat starts so the
  scheduler fallback resolves the selection the app actually uses
- Prefill a usable provider on new scheduled task drafts and add an explicit
  Provider/Model picker with an App-default option and no-provider guardrail
- schedule_task rejects providerless LLM creation when no app default exists
  and labels approval summaries honestly instead of 'Scheduler default'
- Add PiCompactionSessionStore.openChatIfEligible probe; generation runs
  journalless over an in-memory session for rollout-ineligible chats
  (VCC recall omitted, effects never marked durable, store quarantine
  skipped) while openChat keeps its fail-closed throws
- Todo snapshot and compaction resolve benignly for rollout-ineligible chats
- Remote request journal events carry content-free method/route template

@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 critical issues — three documentation/robustness items inline and one open design question; nothing blocks merge.

Reviewed changes

  • Scheduled-task provider resolution — attended chat:start starts seed settings.lastProviderId/lastModel via a new rememberSelection seam (remote/scheduled/bot paths deliberately bypass it); new drafts prefill a pinned provider; the editor gains an App-default / explicit provider + model picker with a soft no-provider guardrail; schedule_task rejects providerless LLM creation when no app default resolves, and approvals show "App default (…)" / "No provider set" instead of the fabricated "Scheduler default".
  • Journalless Pi rollout generationopenChatIfEligible() returns structured rollout reasons while openChat keeps its exact fail-closed throws over the shared openChatInner continuation; ineligible chats run over an in-memory session (VCC recall omitted, effect recovery never marked durable/acknowledged, quarantine gated), chats:todoSnapshot and context-lifecycle compaction resolve benignly, and no v4 journal or v3 migration is ever created for ineligible chats.
  • Remote 4xx evidence — request journal events now carry the HTTP method and a canonical, query-free route template resolved by shape-matching against a per-label template table, with strict diagnostics-grammar admission and tests proving no untrusted path, id, or query is echoed.
  • Suite/test registration updates plus the recovery-plan and papercuts entries. Focused suites (test:scheduled, test:todo, test:compaction, test:diagnostics, test:aiden-remote, and the chat-generation-start cases inside test:subagents) all pass here; the test:subagents failures are the documented pre-existing io_failed environment issue.

ℹ️ Journalless runs present an ephemeral todo UI as "ready"

On an ineligible chat the todo extension still engages: replayTodoState over the empty in-memory journal publishes an availability "ready" snapshot, the agent's todo tool mutates only the per-run in-memory session, and onDurableSnapshot streams every task update into the panel as if it were durable. Between runs chats:todoSnapshot correctly reports unavailable, so the same chat flips between live task-tracking and "Task tracking unavailable" as runs start and stop — and once the rollout advances, the first durable run replays an empty journal, silently discarding the todo state the user watched the agent build during the journalless period. This is an acknowledged trade-off, but presenting ephemeral state through the durable-looking snapshot path invites a lost-work perception; suppressing the todo extension for piJournalless runs (or labeling the panel as session-scoped) would keep the presentation truthful. Open question for the author — no code change is required by this review.

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

Comment thread main/services/aiden-remote-service-main.ts
Comment thread main/services/aiden-remote-router.ts Outdated
Comment thread renderer/components/scheduled-task-editor.tsx
…ontract test, honest pinned model display

- docs/security §7 admits bounded HTTP method + Aiden-owned route templates
  (content-free server constants) while keeping URLs forbidden
- Contract-test every declared route template against the live router
  classification (forward) and scan the matcher source for undeclared route
  shapes (reverse)
- Scheduled-task editor: a pinned model always stays selected and is kept
  selectable when hidden/removed; new-task prefill rejects hidden models;
  switching providers no longer leaks a stale model across providers

@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. All three prior review threads are addressed cleanly in 6047bc4, with contract tests that hold in both directions.

Reviewed changes

  • Threat-model §7 updateddocs/security/aiden-remote-threat-model.md now admits a bounded HTTP method and Aiden-owned route templates (content-free server constants that never reflect a request URL, query string, credential, identifier, or caller-supplied path) while keeping URLs forbidden.
  • Route-template contract coverage addedAIDEN_REMOTE_ROUTE_TEMPLATES and remoteRouteTemplate are exported, and two tests bind them to the router: every declared template is instantiated with grammar-valid tokens and must resolve to its own label and exact routePath against the live handler, and a source scan confined to the handler body proves every string-equality route and …Match regex shape is declared. Both directions pass here.
  • Honest pinned-model display — the editor keeps a pinned model selected even when hidden or removed from the live catalog (prepended until the person replaces it), new-task prefill refuses hidden models, and switching providers can no longer leak a stale model across providers; helper and editor tests cover each case.

Focused suites re-run here are green: aiden-remote-router.test.ts (35/35, including both new contract tests) and the scheduled-task view/editor/schedule-tool group (46/46).

ℹ️ One parallel diagnostics-field enumeration not updated

docs/security/aiden-remote-threat-model.md §7 previously matched the diagnostics sentence in docs/aiden-remote-api-v1.md §8 ("Limits and logging") almost verbatim. §7 is now updated for the new method + route-template fields, but §8 still states diagnostics "may include only a closed route category, outcome or status class, bounded latency, and a stable Aiden-owned error code." If §8 is meant to describe the same records, it should admit the two new fields too; if it is scoped to wire-visible diagnostics only, a sentence saying so would prevent a future security pass from flagging the divergence.

Technical details
# docs/aiden-remote-api-v1.md §8 carries the pre-change field enumeration

## Affected sites
- docs/aiden-remote-api-v1.md:327 — "Diagnostics may include only a closed route category, outcome or status class, bounded latency, and a stable Aiden-owned error code." — the same sentence shape the threat-model §7 carried before 6047bc47; no mention of HTTP method or route templates.

## Required outcome
- Update §8's permitted-fields list to admit the bounded method + Aiden-owned route-template fields added by this PR, or add a sentence scoping §8 to wire-visible diagnostics so the device-local journal (governed by threat-model §7) is not contradicted.

## Open questions for the human
- Is §8 intended as the same content policy as threat-model §7 (then it is stale), or a narrower client-facing promise (then a scope sentence suffices)?

Pullfrog  | View workflow run | Using DeepSeek Flash𝕏

- Status header: generation is never blocked at any stage; advancing is a
  durability decision, not an availability one
- New 'Journalless generation (safety net)' section: engagement conditions
  (pre-activation, deferred v3, rollback env), in-memory semantics, device
  verification steps, and what restores durable journals
- Rollback section: kill switch now yields journalless generation for
  journal-less chats instead of blocked generation
- Implementation notes Phase 7 addendum cross-references the runbook

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

ℹ️ One formatting nit inline — the new runbook and implementation-notes content is faithful to the code; nothing substantive.

Reviewed changes

  • Phase 7 runbook journalless sectiondocs/testing/pi-compaction-phase7-rollout-gates.md now opens with "advancing the stage is a durability decision, not an availability one," extends the Rollback paragraph with the in-memory safety-net behavior, and adds a full "Journalless generation (safety net)" section covering when it engages, run semantics, device verification steps, and the stage-restoration ladder.
  • Implementation-notes paragraphdocs/plans/pi-compaction-memory-upgrade-implementation-notes.md now records that rollout-ineligible chats generate journalless over an in-memory session, pointing at the runbook section.

I cross-checked the new claims against main/services/pi-upgrade-rollout.ts: journal creation is unconditional from migrated_low_risk_chats onward (line 109), v3 migration is cohort-gated with the 500-entry cap applying only at that stage (lines 119-120), the rollback environment blocks new journal creation (line 104), new_chats is the production-default stage, and the three trigger bullets map exactly onto the openChatIfEligible probe outcomes exercised in pi-upgrade-evaluation.test.ts. Content is accurate; only the indentation nit below.

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

Comment thread docs/plans/pi-compaction-memory-upgrade-implementation-notes.md Outdated
The one-second abort deadline flaked twice on this PR's CI (always green
locally and on rerun); five seconds keeps the test honest while tolerating
runner scheduling latency.
@sambitcreate
sambitcreate merged commit 6ce2cd8 into main Sep 11, 2026
6 checks passed
@sambitcreate
sambitcreate deleted the fix/scheduled-provider-and-pi-rollout branch September 11, 2026 16:50
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