From 3cb15ab5a341714edc6ff2f61a9600468f54c394 Mon Sep 17 00:00:00 2001 From: liangxu Date: Tue, 14 Jul 2026 22:01:54 +0800 Subject: [PATCH] docs: update planning artifacts for platform challenge handling Co-Authored-By: Claude Opus 4.8 (1M context) --- findings.md | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ progress.md | 43 +++++++++++++++++++++++++++++++++++++++++++ task_plan.md | 43 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 137 insertions(+), 1 deletion(-) diff --git a/findings.md b/findings.md index 3526cbf..b3b2f25 100644 --- a/findings.md +++ b/findings.md @@ -1,12 +1,62 @@ # Findings & Decisions ## Requirements +- 2026-07-14 follow-up: with exactly one usable account for a platform, the first human-verification challenge must prevent every later local task for that platform from reaching the adapter. +- 2026-07-14: When any account hits a human-verification challenge, retry the current collection with another usable account for the same AI model/platform. +- The client/platform error must always produce a desktop system notification, regardless of whether account failover succeeds. +- If the challenged model/platform has no other usable account, automatically cancel all later tasks for that model/platform. - Verify the repository against the 9-step implementation plan provided by the user. - Use `/Users/liangxu/Documents/test/geo-rankly/docs/superpowers/specs/2026-03-31-admin-web-backend-core-design.md` as the source of truth for the next development slice. - Continue development immediately after identifying the earliest unfinished step. - For desktop AI monitoring, put scheduling authority on the client, keep scraping silent/background-only, allow stale cross-day tasks to be dropped, and avoid fanning one question to all models at once. ## Research Findings +- Screenshot evidence: on client `e83077a0...`, `monitoring_collect:7995` for Doubao failed with `doubao_challenge_required`, while later Doubao task `monitoring_collect:7989` also appeared as failed with the same message. +- Screenshot refinement: the first challenged desktop task `#7224` shows attempt 1, while later `monitoring_collect:7989` shows attempt 0 and the same copied challenge message. This indicates the later task was bulk-terminalized without adapter execution, but recorded as `failed`, making cancellation look like another failed attempt. +- The product-correct state model should be: the triggering task is `failed`; later same-model tasks are `cancelled` in `desktop_tasks` and `skipped` (with a human-verification skip reason) in `monitoring_collect_tasks`, and cancellation events remove them from the local scheduler. +- The missing invariant is client-local: after the only usable account becomes challenged, same-platform tasks already leased or queued locally must be canceled/removed before adapter activation. Server result propagation is asynchronous and cannot be the sole guard. +- `monitor-scheduler` already honors `blockedPlatforms` during selection, and `pumpExecutionLoop` supplies `blockedMonitorPlatforms()`. This supports the screenshot's attempt-0 evidence: the main observed bug is cancellation being represented as failure, not a confirmed second adapter call. +- The scheduler has only single-task removal (`noteMonitorTaskCanceled`); it lacks an atomic "remove all non-active tasks for platform" operation, so local stale signals/legacy leases remain cached until later invalidation. +- Server bulk authorization handling currently sets queued `desktop_tasks` to `failed`, publishes `task_completed`, and sets linked/remaining `monitoring_collect_tasks` to `failed` with `skip_reason = NULL`. Those exact choices produce the misleading repeated-failure rows. +- The queued desktop candidate query also filters `target_account_id`; for monitor tasks this is narrower than model-level cancellation and should be relaxed to client+platform once all local accounts are exhausted, while publish cancellation must remain account-scoped. +- Desktop runtime already removes monitor scheduler entries for both `task_completed` and `task_canceled`; changing the event type improves semantics, while an immediate local platform purge closes the interval before the server event arrives. +- A monitor circuit breaker must be cleared only after an explicit successful account probe/rebind or a later account sync that projects at least one usable account. It should not clear merely because a queued task arrives. +- Server/UI contracts already support `desktop_tasks.status = cancelled`, `monitoring_collect_tasks.status = skipped`, and translated/admin display for skipped/cancelled states, so no schema migration is required. +- Desktop shared event types expose terminal cancellation as runtime `aborted` rather than raw DB `cancelled`; server lifecycle publication must normalize the canceled DB status before sending, or the client event contract becomes inconsistent. +- `publishDesktopTaskLifecycleEvent` currently forwards raw `task.Status`; before choosing a new DB status, confirm the existing desktop cancellation query's canonical status. The client already treats `task_canceled` as removal regardless of status. +- Existing desktop cancellation uses canonical DB/runtime status `aborted`, not `cancelled`. Follow-up monitor desktop tasks should therefore transition to `aborted` and emit `task_canceled`, keeping the shared event contract valid. +- The failed combined server patch made no changes; exact code inspection confirmed the remaining edits are isolated to the authorization bulk helper, its two lifecycle-event loops, and the monitoring bulk SQL. +- Review confirms the triggering task remains failed; only follow-up monitor tasks change to aborted/skipped. Publish authorization behavior remains failed and account-scoped. +- The local circuit breaker participates in both scheduler selection and server lease `platform_ids`, while platform cleanup removes queued/leasing records only and preserves the currently active task. +- Follow-up queued desktop tasks should also set `completed_at = NOW()` when moved to terminal `aborted`, preventing misleading open durations in ops views. +- Full desktop and server regression suites pass after the state transition changes. Existing monitoring outbox/projection code already treats `skipped` as terminal, and ops job projection maps skipped/superseded work to canceled. +- Final diff review found no debug instrumentation or whitespace errors. One remaining audit point is whether the new skip reasons must join projection exclusion lists so canceled human-verification tasks do not affect monitoring coverage metrics. +- Projection audit: only tasks deleted because brand/keyword/question configuration disappeared are excluded from planned samples. Existing `platform_not_accessible` skips remain planned misses, so human-verification cancellations should follow that policy and remain in the denominator. No exclusion-list change is needed. +- Runtime reset and successful account probe/rebind provide explicit circuit-breaker reset points. The breaker should also reconcile after account sync only when health projection reports a usable account. +- 2026-07-14 initial trace: `monitor-account-pool.ts` already filters `challenge_required` accounts and classifies challenge errors as same-platform failover candidates. +- Runtime admission also computes platform-level eligibility and sends it as `platform_ids` during monitor leases; challenge handling therefore needs to distinguish "one account blocked" from "no usable account remains" before excluding/canceling a model. +- Current desktop/server cancellation APIs cancel one current task/lease. No explicit batch operation for all later tasks of one AI platform/model was found in the first search. +- Electron main bootstrap already owns tray-balloon behavior, while current monitor challenge handling primarily writes runtime activities and renderer issue rows; a reusable main-process system notification seam is needed for all runtime client errors. +- The account-pool classifier already returns `challenge_required`/`risk_control` failover reasons and the execution loop references candidate selection around `runtime-controller.ts:3273`; focused inspection must verify notification timing and whether health projection removes only the failed account. +- Server final-result handling already has two bulk authorization-failure paths: queued `desktop_tasks` for the same target account/platform, and same-day `monitoring_collect_tasks` for the same client/platform. Both are activated only when the final submitted result is non-retryable AI authorization failure. +- The monitoring bulk path sets later model tasks to `failed` with an explicit `blocked_by_authorization_failure` payload; product wording says "cancel", so final verification must decide whether the existing terminal failure state is the intended schema-compatible cancellation representation or needs a distinct state transition. +- Hypothesis 1 was falsified by code-path inspection: `executeMonitorTaskWithAccountPool` snapshots all usable same-platform accounts before attempts, then continues to the next account after the first account is marked challenged/cooling. The health change does not remove the already-selected alternate from the current loop. +- Hypothesis 2 is confirmed: challenge/account access handling only calls `recordActivity`; there is no runtime-to-Electron system-notification callback, so an intermediate account failure remains invisible outside the activity/issue UI even when failover succeeds. +- Hypothesis 3 is confirmed at implementation level: a final non-retryable AI authorization result invokes server bulk failure for remaining same-day platform tasks. The desktop task schema supports `cancelled`, but monitoring collection workflow uses `failed` as the terminal authorization-block state; preserving that state avoids introducing an incompatible cross-database status without product benefit. +- Required regression seams: desktop pure policy/helper coverage for "notify on challenged account even when an alternate remains", plus server SQL-contract coverage proving platform-wide remaining task failure is scoped to the same client/platform/business date and triggered only by non-retryable AI authorization. +- Implementation boundary: add a runtime-to-Electron notification sink, route every `danger` activity through it, and explicitly notify account challenge/risk-control transitions because those remain `warn` while same-platform failover continues. +- Use native Electron notifications on macOS/Linux and the existing tray balloon path on Windows; add short-window duplicate suppression in the runtime notification sink to avoid repeated loop errors flooding the user. +- Desktop tests run under Vitest's default discovery (no desktop-local Vitest config), so the notification sink should remain Electron-free and unit-testable; only `bootstrap.ts`/`tray.ts` should depend on Electron notification APIs. +- Existing server tests do not directly mention the authorization bulk-failure helpers, confirming the cancellation/terminalization scope currently has no focused regression lock. +- The new runtime notification test was red before implementation (`Cannot find module './runtime-system-notification'`) and now passes together with the existing same-platform account-pool tests. +- Native notification delivery is isolated in `tray.ts`: Windows prefers the initialized tray balloon and falls back to Electron `Notification`; macOS/Linux use Electron `Notification` when supported. +- Existing callback tests already prove authorization failures are non-retryable and that explicit target client wins over leased executor. The untested part is the bulk SQL scope (`business_date + platform + client`, excluding the current task), not the failure disposition itself. +- The existing `captureDailyMonitorTaskTx` test double implements `pgx.Tx` and records SQL/arguments, so `failRemainingMonitoringTasksForAuthorizationFailure` can be exercised directly without a database or production refactor. +- Final result ingest calls the platform-wide bulk failure only after a failed result is not requeued. Intermediate account errors never reach this server method because the desktop account loop submits only its final result; this is the key invariant that allows alternate-account success without canceling later tasks. +- The bulk-failure method accepts a `monitoringCollectTask` containing tenant, business date, platform, execution owner, and target/leased client fields, so the SQL-scope regression can assert both query predicates and exact bound arguments. +- `doubao_challenge_required` is recognized as an authorization failure by the server's `_challenge_required` suffix rule, and an explicit `retryable=false`/`failure_category=ai_platform_authorization` final error activates the bulk path. +- Final behavior: one challenged account is immediately notified and excluded while the current task continues on another usable same-model account; if the account pool is exhausted, the final non-retryable result terminalizes later same-day tasks for that client/model through the existing server bulk path. +- Final automated verification passed. Native notification center display itself remains a host-OS integration caveat, but the Electron API path compiled in the production bundle. - Current 2026-05-07 publish-management request targets `apps/admin-web`: the user wants a new content-management page showing publish title, account/platform, status, task info, time, and actions for external link, opening the desktop client workbench, and retrying. - Desktop client already has `PublishManagementView.vue` with the desired table shape and action semantics, but it calls Electron IPC (`window.desktopBridge`). Admin-web needs tenant-authenticated HTTP endpoints instead. - Existing backend desktop endpoints are `/api/desktop/publish-tasks` and `/api/desktop/publish-tasks/:id/retry`, but they require desktop client token auth. Tenant API currently exposes publish-job creation only, not publish task list/retry. @@ -135,6 +185,8 @@ ## Issues Encountered | Issue | Resolution | |-------|------------| +| A zsh glob for `apps/desktop-client/vitest.config.*` had no matches during read-only inspection | Switched to `rg --files`/explicit paths; no repository state changed | +| First planning-file patch used a stale placeholder row and failed verification | Re-read the exact table context and applied a narrower patch | | Repository is fully untracked in git status output | Avoid relying on git history; verify via filesystem and test commands instead | | `make sqlc-generate` fails before any code is generated | Repair the `sqlc.yaml` schema path and then build repository wrappers around generated queries | | `go run` migrate initially failed with `unknown driver postgres` | Added `-tags postgres` to the Makefile-managed migrate command | diff --git a/progress.md b/progress.md index 50b7a38..cbefe85 100644 --- a/progress.md +++ b/progress.md @@ -1,5 +1,48 @@ # Progress Log +## 2026-07-14T18:45:00+08:00 - Local same-platform challenge circuit breaker started + +- User screenshot disproved the previous end-to-end assumption: after `monitoring_collect:7995` hit `doubao_challenge_required`, later Doubao task `7989` still attempted collection on the same single account. +- Added Phases 66-68 to reproduce the local queue race, add a platform circuit breaker, and verify it without changing other-platform scheduling or multi-account failover. +- Initial direction: cancel/remove already leased or queued same-platform work locally as soon as the last usable account becomes challenged; do not wait for server bulk status propagation. +- Refined the screenshot diagnosis: later Doubao row `7989` has attempt 0, so it was not proven to reach the adapter; it was bulk-marked failed with the original challenge error, which is still the wrong cancellation state. +- Added two red regressions. Desktop scheduler fails because no platform-level pending-task removal function exists; server test fails because later tasks are explicitly set to `failed` with no `skip_reason`. +- Phase 66 is complete and Phase 67 implementation is in progress. +- Desktop scheduler/runtime circuit-breaker changes applied successfully. The first combined server patch failed context verification and made no server edits; implementation continues with smaller exact-context patches. +- Scheduler/account-pool focused tests now pass. First typecheck found one redundant event payload field; the server regression reached the new `skipped` SQL and now only needs updated argument assertions. +- Implemented local platform circuit breaking, atomic same-platform pending-task removal, manual probe/rebind recovery, model-wide monitor cancellation across account targets, canceled lifecycle events, and skipped monitoring follow-ups. +- Focused desktop tests pass (3 files / 12 tests), desktop typecheck passes, focused server authorization-cancellation tests pass, and `git diff --check` passes. +- Full verification passes: desktop 31 files / 213 tests, desktop typecheck, desktop production build, and server-wide `go test ./...`. +- Final diff review found no debug instrumentation, whitespace errors, or unintended projection changes. +- Phase 68 is complete. Live captcha triggering was not repeated against the user's account; the screenshot sequence is covered deterministically at the local scheduler and server terminal-state boundaries. +- Root cause: follow-up tasks were already prevented from adapter execution in the shown case (`attempt 0`) but were bulk-recorded as failed with the original challenge message, and the client lacked immediate platform-wide queue purging. The fix addresses both the misleading state and the local race. +- Planning completion check still reports historical unfinished phases 11/12, 17, and 48/49 from earlier roadmap work; current follow-up phases 66-68 are complete and unrelated roadmap state was preserved. + +## 2026-07-14T00:00:00+08:00 - Human-verification failover and alert hardening started + +- Added Phases 63-65 for challenge reproduction, account-scoped failover, model-level cancellation, and desktop system notifications. +- Recovered the latest multi-account context: same-platform account pooling already exists, while Doubao challenge handling currently blocks the platform at admission and surfaces only activity/page state rather than a guaranteed OS notification. +- Added `runtime-system-notification.test.ts` for danger alerts, forced account-challenge alerts, per-account deduplication, and notification failure isolation. +- Red test confirmed the missing seam: Vitest fails because `runtime-system-notification` does not exist yet. +- Implemented the Electron-free notification policy/sink with 30-second per-error deduplication and exception isolation. +- Wired all `danger` runtime activities plus challenge/risk-control account transitions into the sink. +- Added native delivery through Windows tray Toast and Electron notifications on macOS/Linux, configured once during desktop bootstrap. +- Focused notification/account-pool tests pass (2 files, 8 tests); desktop Vue/TypeScript typecheck passes. +- Added a direct server regression test for final `doubao_challenge_required` handling. It verifies same-tenant/date/model/client scoping, current-task exclusion, terminal source statuses, bound arguments, and the authorization-block payload. +- The focused server test set passes, including existing non-retryable and target-client precedence checks. +- Full verification passes: desktop 31 files / 212 tests, desktop typecheck, desktop production build, server `internal/tenant/app` tests, and `git diff --check`. +- Final diff review confirmed ordinary recoverable warnings stay in the activity feed, while `danger` errors and account challenge/risk-control transitions produce native system notifications. +- Phase 65 is complete. Automated coverage cannot click the host OS notification center, but the Electron native notification path passes typecheck and production bundling. +- The planning skill completion check reports historical unfinished phases 11/12, 17, and 48/49 from earlier work; current request phases 63-65 are all complete and were not used to rewrite unrelated roadmap state. +- Files changed for this scope: + - `apps/desktop-client/src/main/runtime-system-notification.ts` (created) + - `apps/desktop-client/src/main/runtime-system-notification.test.ts` (created) + - `apps/desktop-client/src/main/runtime-controller.ts` + - `apps/desktop-client/src/main/bootstrap.ts` + - `apps/desktop-client/src/main/tray.ts` + - `server/internal/tenant/app/monitoring_callback_service_test.go` + - planning files + ## Session: 2026-04-30 ### Phase 37: k3s Deployment Foundation diff --git a/task_plan.md b/task_plan.md index e665b52..a167cc0 100644 --- a/task_plan.md +++ b/task_plan.md @@ -4,7 +4,7 @@ Continue the desktop AI monitoring implementation by first adding a client-side local scheduler with durable queueing and adaptive execution policy, then adding a reusable hidden Playwright CDP execution layer that attaches to Electron Chromium and reuses existing account partitions. ## Current Phase -Phase 60 +Phase 68 ## Phases ### Phase 1: Progress Verification @@ -419,6 +419,42 @@ Phase 60 - [x] Remove temporary visual artifacts and record results - **Status:** complete +### Phase 63: Human-Verification Failure Reproduction +- [x] Trace account-level challenge classification, same-model failover, task admission, and desktop notifications +- [x] Add deterministic regression coverage for challenge failover, single-account cancellation, and system alerts +- [x] Confirm the tests reproduce the current incorrect behavior +- **Status:** complete + +### Phase 64: Account Failover, Cancellation, And Alerts +- [x] Keep challenge blocking scoped to the failed account when another same-model account is usable +- [x] Cancel all later tasks for the model when no alternate usable account remains +- [x] Show a system notification for every client/platform error while preserving in-app diagnostics +- **Status:** complete + +### Phase 65: Verification And Delivery +- [x] Run focused desktop/server regression tests and type/build checks +- [x] Confirm no challenge path can silently continue on the failed account +- [x] Record final behavior, changed files, and residual live-verification limits +- **Status:** complete + +### Phase 66: Local Queue Challenge Reproduction +- [x] Trace monitor scheduler activation, local queue cancellation, and server task invalidation after a challenge +- [x] Build a deterministic failing test with two queued same-platform tasks and one usable account +- [x] Confirm the later task is currently represented as another failure instead of cancellation +- **Status:** complete + +### Phase 67: Platform Circuit Breaker +- [x] Immediately block the challenged platform when no alternate account remains +- [x] Remove/cancel queued and deferred local tasks for that platform before adapter execution +- [x] Keep other platforms running and preserve same-platform failover when another account is usable +- **Status:** complete + +### Phase 68: Circuit-Breaker Verification +- [x] Run focused scheduler/runtime tests, full desktop tests, typecheck, build, and server checks +- [x] Verify the screenshot sequence cannot recur in the deterministic harness +- [x] Record final behavior and any live-environment caveat +- **Status:** complete + ## Key Questions 1. How should the desktop client defer and locally optimize monitor-task execution without violating one-task-per-platform serialism? 2. What is the smallest durable local cache that allows same-day resume while safely dropping stale next-day monitor tasks? @@ -481,6 +517,11 @@ Phase 60 ## Errors Encountered | Error | Attempt | Resolution | |-------|---------|------------| +| Runtime invalidation payload included unsupported `platform` field | 1 | Remove the redundant field; account ID and message already identify the health event | +| Server regression still expected old literal skip reason and 10 SQL args | 1 | Update assertions to the new parameterized skip reason/cancellation payload and 11 args | +| Authorization bulk-cancellation patch context did not match the existing SQL argument block | 1 | Split the server change into smaller exact-context patches | +| zsh no-match on optional Vitest config glob | 1 | Use `rg --files` or explicit paths instead of a shell glob | +| Planning-file patch context did not match the existing error table | 1 | Re-read the exact section and apply a narrower patch | | None yet | 1 | N/A | | `make sqlc-generate` failed because `sqlc.yaml` points to `../../migrations/`, which resolves to `server/internal/migrations` | 1 | Fix schema path to repository-relative `../../../migrations/` and then generate code | | `make lint` initially failed on several `errcheck` and one `ineffassign` warning | 1 | Fixed transaction defers, the middleware test Redis set call, and the dead increment in `article_service.go` |