fix: low-risk bug hunt — Tier A (11 verified fixes)#63
Open
ystyleb wants to merge 4 commits into
Open
Conversation
From an automated bug hunt (parallel code-reviewers + Codex second opinion), all verified low/zero-risk: - prompting/contextSelection: sortOptionalBlocks dropOrder comparator was inverted (leftDrop-rightDrop), so the group meant to be dropped FIRST was selected/kept first and groups absent from dropOrder were dropped first. - director/recoverableRange: Boolean(progress?.recoverableRange) was always true (the field is always a non-null object); resumability now checks startOrder != null. - director/resolveCurrentScopedChapter: removed a dead duplicate status === "running" lookup (the first `active` search already covered it). - workflow/recordCheckpoint: the only progress update path lacking the Math.max(existing.progress, ...) guard could regress task progress; aligned with the other update sites. (Verified no caller uses checkpoints to lower progress — recovery/requeue use separate paths.) - planner/buildWindowOrders: surrounding-mode fallback counted duplicate clamped chapter numbers against windowSize, returning fewer chapters when the anchor was near chapter 1. Track unique numbers with a Set; exported for test. - settings/ProviderBalanceService: missing `await` on response.json() let the finally block clear the 12s abort timer before the body was read. - task/NovelWorkflowTaskAdapter: tautology `type === X ? X : X` laundered any untrusted action.type into "open_structured_outline"; now rejects unknown. Tests (TDD red→green): contextSelectionDropOrder, providerBalanceTimeout (scoped to its own abort handle to avoid global-mock pollution), replanWindowOrders, and chapter-execution recover guards. Full server suite: 878 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the same low-risk bug hunt, all verified low/zero-risk: - cover/NovelCoverDialog: the delete (and set-primary) buttons used deleteAssetMutation.variables === asset.id without isPending. react-query keeps `variables` after a mutation settles, so a FAILED delete left the button permanently disabled / stuck on "删除中...". Gate on isPending too. - lib/directorTaskNotice: tautology `type === X ? X : X` accepted any untrusted action.type and laundered it into "open_structured_outline"; now rejects unknown action types (mirrors the server adapter fix). - worlds/WorldWorkspace: onInjectLibraryField/Structure fired useWorldLibraryItem().then() with no .catch — API errors were silently swallowed. Surface via toast.error. - creativeHub/useCreativeHubRuntime: loadThread().then() had no .catch, so a network error left the UI stuck on the loading state. Catch, unstick (setThreadStateLoaded(true)), and toast the error. - writingFormula/useWritingFormulaCreateFlow: refreshStyleData().then() had no .catch, so a refresh failure dropped the auto-open callback. Still notify the user (the profile is already saved server-side). Test (TDD red→green): directorTaskNotice.test.mjs (keeps valid action, rejects unknown type). The .catch/disabled-state fixes are framework glue not covered by the node test harness (no React render); verified by typecheck. Full client suite: 36 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Records the full bug-hunt result (30 candidates -> 21 confirmed -> Codex re-grading) and the Tier A/B/C/D classification, including: - Tier A: the 12 low/zero-risk items (11 implemented this round). - Tier B: real bugs needing care (e.g. validate middleware drops query/params coercion under Express 5 read-only req.query). - Tier C: items Codex downgraded to non-bugs (e.g. buildPipelineStageProgress is correct as-is; "fixing" it would introduce a bug). - Tier D: new findings from Codex (NovelEdit artifact_delta whitelist, etc). - A9 (llmStore maxTokens reset) deferred pending product-intent confirmation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…a number The validate middleware validates query schemas but does not write the coerced result back to req.query (and Express 5's req.query is a read-only getter, so it cannot simply be reassigned in the middleware). This route trusted `req.query as z.infer<...>`, so chapterOrder — declared z.coerce.number() — arrived as the raw string "5". getPayoffLedger guards on `typeof chapterOrder === "number"`, so it silently dropped the string and returned the default chapter's ledger instead of the requested one. Re-parse req.query in the handler (matching the dominant pattern elsewhere, e.g. novelBaseRoutes/rag/autoDirectorFollowUps) so the z.coerce applies. This is the surgical, zero-blast-radius fix for the single endpoint that actually manifests the shared-middleware flaw (Tier B "B1"); the broader middleware hardening (write coerced query/params back under Express 5) is left as a separate follow-up. Verified the flaw is otherwise latent: all other coercing query routes already re-parse, and the rest only cast string params/enums. Test (TDD red->green): payoffLedgerQueryCoercion — service receives a number, undefined when the param is absent. Full server suite: 880 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
对全代码库做了一次低/零风险 bug 排查：9 路
code-reviewer并行扫描 → 每候选对抗式验证 → Codex 独立第二意见 → 关键项作者亲自读码裁决。原始 30 候选 → 去重 21 确认 → Codex 复审后分级（详见docs/bug-hunt-low-risk-proposals.md）。本 PR 实现 Tier A 的 11/12（A9 暂缓——产品意图待确认）。全部为机械式、意图明确、低/零风险修复。
改动
server（commit 1）
prompting/core/contextSelection.tsdropOrder比较器方向反了——"应最先丢弃"的组反被优先保留。改rightDrop - leftDrop。director/.../directorExecutionStepModules.tsBoolean(progress?.recoverableRange)恒为真（该字段永远是非空对象）→ 改判startOrder != null。director/.../directorWorkflowStepShared.tsresolveCurrentScopedChapter里重复的status==="running"死代码。workflow/NovelWorkflowApplicationService.tsrecordCheckpoint是唯一缺Math.max进度防回退的路径（其余 6 处都有）。已确认无调用方依赖"回退进度"。planner/replanDecision.tsbuildWindowOrderssurrounding 兜底把重复 clamp 值计入 windowSize，anchor 靠近第 1 章时少返章节。改用 Set 去重，export 供测。settings/ProviderBalanceService.tsresponse.json()漏await——finally的clearTimeout在读 body 前触发，慢响应逃过 12s 超时。task/adapters/NovelWorkflowTaskAdapter.tstype===X ? X : X把不可信action.type洗白成合法值；改为拒绝未知类型。client（commit 2）
cover/NovelCoverDialog.tsxmutation.variables === id未配isPending——删除失败后按钮永久禁用 / 卡"删除中..."。lib/directorTaskNotice.tsworlds/WorldWorkspace.tsxuseWorldLibraryItem().then()漏.catch——API 错误被静默吞掉。creativeHub/useCreativeHubRuntime.tsloadThread().then()漏.catch——网络错误时 UI 永久卡 loading。writingFormula/useWritingFormulaCreateFlow.tsrefreshStyleData().then()漏.catch——刷新失败丢失自动打开回调。docs（commit 3）
docs/bug-hunt-low-risk-proposals.md：完整排查结果 + Tier A/B/C/D 分级（含 Codex 把buildPipelineStageProgress等降级为"非 bug、勿改"的记录）。测试与验证
contextSelectionDropOrder、providerBalanceTimeout（绑定自身 abort handle 防全局 mock 污染）、replanWindowOrders、directorTaskNotice.test.mjs+ chapter-execution recover guard。暂缓 / 后续
llmStore的maxTokens换选择时重置）：可能是有意产品决策（maxTokens 模型相关），落入意图灰区，未改。validate中间件在 Express 5 下丢弃 query/params coercion，系统性）/ Tier C（非 bug）/ Tier D（Codex 新发现）。🤖 Generated with Claude Code