docs(product): define Card-to-Run collaboration contract - #15
Conversation
|
Important Review skippedToo many files! This PR contains 110 files, which is 10 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (110)
You can disable this status message by setting the 📝 WalkthroughWalkthrough新增 Kanban 执行 occurrence、事件 cursor、幂等回执、阻塞重试、writeback、执行历史 API 和 Console 展示。新增相关存储迁移、集成测试、设计原型、产品需求及执行回执合同。 ChangesKanban execution contract
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/22-jtype-agent-work-prd.md`:
- Around line 417-421: 更新该 Kanban claim/occurrence 设计说明，明确使用 Service Kanban
binding id 作为 claim key，并说明不同 binding 之间必须隔离。若继续采用 `(automation_id,
document_id)` 作为锚点，则改为明确 claim/occurrence 存储需与 Automation 标识隔离，确保同一 Card 被多个
binding 使用时不会合并。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fa90549-ff06-4cdd-8eaf-7084a0640256
📒 Files selected for processing (2)
docs/22-jtype-agent-work-prd.mddocs/README.md
| claim 是 `(automation_id, document_id)` 的永久关联锚点，不再承担“一生只跑 | ||
| 一次”的语义。occurrence 表示一次有效进入触发列，使用稳定 transition event | ||
| key 幂等。首次进入创建 claim + occurrence；离开再进入复用 claim 并创建新 | ||
| occurrence。active occurrence、同事件 replay、仍在列内的正文编辑不创建新 | ||
| occurrence。 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -F 'docs/22-jtype-agent-work-prd.md' || true
echo "== relevant sections =="
if [ -f docs/22-jtype-agent-work-prd.md ]; then
nl -ba docs/22-jtype-agent-work-prd.md | sed -n '1,220p'
echo "---- 380-440 ----"
nl -ba docs/22-jtype-agent-work-prd.md | sed -n '380,445p'
fi
echo "== search automation_id/kanban_binding/context =="
rg -n "automation_id|kanban_binding|claim|occurrence|Service Kanban|Card|project|tenant|binding" docs/22-jtype-agent-work-prd.md AGENTS.md 2>/dev/null || trueRepository: cnjack/cloud
Length of output: 264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant section 1-220 =="
awk 'NR>=1 && NR<=220 { printf "%5d %s\n", NR, $0 }' docs/22-jtype-agent-work-prd.md
echo "== relevant section 380-445 =="
awk 'NR>=380 && NR<=445 { printf "%5d %s\n", NR, $0 }' docs/22-jtype-agent-work-prd.md
echo "== search definitions/usages =="
grep -nE "automation_id|kanban_binding|claim|occurrence|Service Kanban|Card|scope|binding|tenant|project" docs/22-jtype-agent-work-prd.md AGENTS.md 2>/dev/null || trueRepository: cnjack/cloud
Length of output: 25018
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== docs/design/plugin-platform.md exists =="
git ls-files | grep -F 'docs/design/plugin-platform.md' || true
if [ -f docs/design/plugin-platform.md ]; then
echo "== Service Kanban / automation id mentions =="
grep -nE "Service Kanban|Kanban|board binding|binding|automation_id|automation as|automations|automation" docs/design/plugin-platform.md || true
echo "== relevant section approx around Kanban/service =="
grep -n -C 8 "Kanban\|board\|binding\|automation_id\|Service" docs/design/plugin-platform.md || true
fi
echo "== 22 additional mentions with context =="
grep -n -C 4 "automation_id\|Kanban binding\|board binding\|binding" docs/22-jtype-agent-work-prd.md || trueRepository: cnjack/cloud
Length of output: 28356
Use the Service Kanban binding id for Kanban claim keys.
Service Kanban binds a Card to a Service Kanban binding with its own stable identity, but the Kanban claim anchor is already defined as (automation_id, document_id). Document the Kanban binding key with cross-binding isolation, or keep claim/occurrence storage separate from Automation identifiers so the same Card used by multiple bindings cannot be conflated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/22-jtype-agent-work-prd.md` around lines 417 - 421, 更新该 Kanban
claim/occurrence 设计说明，明确使用 Service Kanban binding id 作为 claim key，并说明不同 binding
之间必须隔离。若继续采用 `(automation_id, document_id)` 作为锚点，则改为明确 claim/occurrence 存储需与
Automation 标识隔离，确保同一 Card 被多个 binding 使用时不会合并。
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Nitpick comments (19)
orchestrator/internal/store/plugin_kanban_occurrence.go (1)
563-566: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winBound the unique-violation retry.
Both conflict paths roll back and call
s.ObservePluginKanbanCard(ctx, in)recursively with no depth limit. Each level opens a new transaction. A repeated race, or a conflicting row that the retry cannot read, produces unbounded recursion instead of a returned error. Convert the recursion into a bounded retry loop, for example two attempts, and return the error after the last attempt.Also applies to: 640-643
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/store/plugin_kanban_occurrence.go` around lines 563 - 566, Replace the recursive ObservePluginKanbanCard calls in both unique-violation paths with a bounded retry loop, allowing at most two transaction attempts. Preserve rollback and retry behavior for the first conflict, but return the final error when the retry limit is reached; update the surrounding ObservePluginKanbanCard flow consistently.orchestrator/internal/domain/plugins.go (1)
341-377: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider typed constants for
Outcome,ReceiptPhase, andWritebackState.
Stateis a typed enum, but the three sibling enum-like fields stay plainstring. The stores write literals such as"pending","complete","not_required","accepted", and"terminal"in bothMemStoreandPGStore. A typo compiles and only fails at the PostgreSQL CHECK constraint, or silently diverges inMemStore. Declaring named constants next toKanbanOccurrenceStatewould make the memory and PostgreSQL paths use one source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/domain/plugins.go` around lines 341 - 377, Define typed enums and named constants for Outcome, ReceiptPhase, and WritebackState alongside KanbanOccurrenceState, covering the existing values used by MemStore and PGStore such as pending, complete, not_required, accepted, and terminal. Update PluginKanbanOccurrence and all store assignments/comparisons to use these types and shared constants instead of raw strings.orchestrator/internal/store/memory.go (1)
3307-3323: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant pointer re-check after the equality guard.
Line 3339 already requires
claim.LatestOccurrenceID == occurrenceID, so the extraclaim.LatestOccurrenceID != ""test at line 3345 only excludes the legacyoccurrenceID == ""call shape.MarkPluginKanbanWritebackUnavailableomits that second test and reads the map directly, which is clearer. The stale-occurrence guard itself is correct and matchesTestPluginKanbanStaleWritebackCannotCompleteNewOccurrence.Also applies to: 3325-3362, 3364-3401
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/store/memory.go` around lines 3307 - 3323, Remove the redundant claim.LatestOccurrenceID != "" guard in the occurrence lookup within the affected writeback methods, matching MarkPluginKanbanWritebackUnavailable’s direct map access pattern. Preserve the existing claim.LatestOccurrenceID == occurrenceID equality guard and pending WritebackState check, including the corresponding logic in the additional affected ranges.orchestrator/internal/store/plugins.go (1)
738-780: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRemove the per-row Run and claim lookups.
The outer query already joins
runsandautomation_kanban_claimsto select the occurrences. The loop then issuesGetRunplus one claim query for every returned occurrence, so a reconcile tick costs2Nextra round trips and can read a row that changed after the first query. Select the claim and run columns in the same statement, alongsidequalifiedPluginKanbanOccurrenceCols, and scan all three structs in one pass.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/store/plugins.go` around lines 738 - 780, Update ListPluginKanbanRunsAwaitingWriteback to select the required run and claim columns alongside qualifiedPluginKanbanOccurrenceCols in its existing joined query, then scan all three records in the rows.Next loop using the established scan helpers. Remove the per-occurrence GetRun and scanPluginKanbanClaim queries, while preserving the existing error wrapping and PluginKanbanWriteback assembly.orchestrator/internal/store/store.go (1)
549-565: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the contract for the new Kanban methods.
Every other block in this interface states its idempotency and error mapping, for example
MarkKanbanWritebackat line 700. The 17 new methods carry no comments, yet their semantics are subtle and currently differ betweenMemStoreandPGStore:
CreatePluginKanbanOccurrenceRunreturns(false, nil)when the occurrence already owns a Run.MarkPluginKanbanWritebackandMarkPluginKanbanWritebackUnavailablereturn(false, nil)for a staleoccurrenceID, and an emptyoccurrenceIDselects the legacy claim-only shape.MarkPluginKanbanOccurrenceReceiptreturnsErrConflictwhen the stored phase differs.SetPluginKanbanOccurrenceBlockedreturnsErrConflictonce a Run is attached.AdvancePluginKanbanTriggeris a compare-and-set onpreviousCursorand refuses a second bootstrap stamp.Recording these rules here gives both implementations one authoritative contract to be checked against.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/store/store.go` around lines 549 - 565, Add interface comments documenting the idempotency and error contracts for the new PluginKanban methods, using existing comments such as MarkKanbanWriteback as the style reference. Explicitly record CreatePluginKanbanOccurrenceRun’s existing-run result, stale and legacy-selection behavior for both writeback methods, ErrConflict conditions for receipt and blocked updates, and AdvancePluginKanbanTrigger’s compare-and-set and one-time bootstrap semantics; ensure the comments cover all 17 new methods without changing implementation behavior.orchestrator/internal/store/plugin_kanban_occurrence_test.go (1)
288-341: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the two cases that would have caught the store divergences.
The suite covers replay, atomic claim, stale writeback, pagination, retention, and receipt phases. Two cases are missing, and both map to defects reported in this cohort:
- Call
SetPluginKanbanClaimRunon a claim whoseLatestOccurrenceIDis already set, then assert the occurrence reachesqueuedwith the Run attached.MemStorecurrently leaves the occurrence atreceivedwith an emptyRunID, whilePGStoreattaches the Run.- Call
MarkPluginKanbanWritebacktwice with identical arguments and assert the second call returns(false, nil).PGStorecurrently returns an error for the empty-occurrenceIDshape.Also prefer
errors.Is(err, ErrNotFound)over the direct comparison at line 363, so the assertion survives future error wrapping.Also applies to: 343-366
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/store/plugin_kanban_occurrence_test.go` around lines 288 - 341, Extend the plugin Kanban occurrence tests with cases for SetPluginKanbanClaimRun on a claim with LatestOccurrenceID already set, asserting the occurrence becomes queued and attaches the Run, and for repeated MarkPluginKanbanWriteback calls with identical arguments, asserting the second returns false with no error, including an empty occurrenceID where applicable. Update the existing not-found assertion near the referenced comparison to use errors.Is with ErrNotFound.orchestrator/internal/jtype/board_resolve_test.go (1)
129-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative-path coverage for
GetBoardByConfigID.The test covers only the happy path. Three behaviors carry more risk and are untested:
- An unknown
configIDmust returnErrDocNotFound.- An empty
configIDmust returnErrDocNotFoundwithout any HTTP request.- A
.boarddocument that holds invalid board JSON must not prevent resolution of a later, valid board document. This case demonstrates the abort behavior flagged inorchestrator/internal/jtype/client.goLine 378-390.Add a workspace listing with one malformed
.boarddocument followed by the configured one, and assert that the configured board resolves.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/jtype/board_resolve_test.go` around lines 129 - 149, Expand GetBoardByConfigID test coverage with cases for an unknown configID returning ErrDocNotFound and an empty configID returning ErrDocNotFound without making an HTTP request. Also add a workspace listing containing a malformed .board document before the valid configured document, and assert the valid board still resolves rather than aborting on the malformed content.design/assets/kanban-agent-executions.js (1)
37-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueApply the initial state once on load.
The listeners only call
setStateon click. Until the first click,aria-pressed, the panelhiddenvalues, the policy tone, and the policy text come from the static markup. Those five places must agree with each other indesign/kanban-agent-executions.html, or the prototype opens in a mixed state.Derive the initial state from the pressed button and call
setStateonce.♻️ Optional cleanup
buttons.forEach((button) => { button.addEventListener("click", () => setState(button.dataset.kaStateButton)); }); + + const initial = buttons.find((button) => button.getAttribute("aria-pressed") === "true") ?? buttons[0]; + if (initial) setState(initial.dataset.kaStateButton); })();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design/assets/kanban-agent-executions.js` around lines 37 - 40, Update the initialization flow around the button click listeners to derive the initial state from the button marked pressed in the static markup, then call setState once on load before user interaction. Preserve the existing click handlers while ensuring aria-pressed, panel visibility, policy tone, and policy text are synchronized immediately.orchestrator/internal/jtype/client.go (1)
369-377: 🚀 Performance & Scalability | 🔵 TrivialBoard resolution costs one request per
.boarddocument on every tick.
validatePluginKanbanBoardcallsGetBoardByConfigIDonce per enabled Kanban Automation per tick. Each call performs oneListDocumentsrequest plus oneGetDocumentrequest per.boarddocument. Workspaces with many boards and many Automations produce a large per-tick request fan-out against jtype.Consider caching the configID-to-document-ID mapping in the poller, and re-resolving only when the cached document no longer matches the configured ID.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/jtype/client.go` around lines 369 - 377, The GetBoardByConfigID path currently lists and fetches every board document for each validation call; add poller-level caching from configID to document ID around validatePluginKanbanBoard, reuse cached mappings across ticks, and re-resolve through GetBoardByConfigID only when the cached document is missing or no longer matches the configured ID.orchestrator/internal/reconciler/kanban_test.go (1)
290-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSeed the deleted-card test through the occurrence API.
TestPluginKanbanWritebackUsesInstallationWorkspacewas migrated toObservePluginKanbanCardplusCreatePluginKanbanOccurrenceRunat Lines 200-213. This test still usesEnsurePluginKanbanClaimplusSetPluginKanbanClaimRun.That legacy path synthesizes an occurrence with a
legacy:event key and noEntryColumnhistory, so the test asserts deleted-card behavior against a shape the poller no longer produces. Use the same seeding as the sibling test so the coverage matches production.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/reconciler/kanban_test.go` around lines 290 - 297, Update TestPluginKanbanWritebackUsesInstallationWorkspace to seed the deleted-card occurrence through ObservePluginKanbanCard and CreatePluginKanbanOccurrenceRun, matching the setup used by the sibling test around the existing occurrence API. Replace the EnsurePluginKanbanClaim and SetPluginKanbanClaimRun calls so the test includes the production-shaped occurrence and EntryColumn history.orchestrator/internal/kanban/poller.go (1)
297-316: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCap the head-discovery loop.
pluginAutomationEventHeadwalks the entire event feed in pages of 100 untilHasMoreis false. The termination guards are correct: the loop only continues whenNextSequencestrictly exceeds the previous head, so a stalled or backwards feed exits with an error.The cost is unbounded for a board with a long event history, and all pages are pulled inside a single tick. Consider an iteration cap that returns the highest observed sequence, or a dedicated head query if jtype exposes one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/kanban/poller.go` around lines 297 - 316, Cap the pagination loop in pluginAutomationEventHead so a single tick cannot walk an unbounded event history. Track iterations or pages, stop at a reasonable configured limit, and return the highest observed head when the cap is reached; preserve the existing backwards-cursor and non-advancing-feed error guards.orchestrator/internal/reconciler/reconciler.go (1)
1096-1110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the accepted-receipt body into one shared builder.
This block rebuilds the accepted receipt body that
orchestrator/internal/kanban/poller.goalready builds at Lines 637-642. The two copies must stay consistent, because the poller deduplicates on the marker alone and never compares bodies.They already diverge: the poller appends the Run link unconditionally, so an empty
consoleURLyields"Run: /runs/<id>". This code guards withif r.consoleURL != "".Move the body construction next to
jtype.KanbanReceiptMarker, and call it from both packages.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/reconciler/reconciler.go` around lines 1096 - 1110, Extract the accepted-receipt body construction into one shared builder located next to jtype.KanbanReceiptMarker, including the poller’s unconditional Run link behavior. Replace the local construction in the reconciler block and the duplicate logic in poller.go with calls to that shared builder, passing the service name, model, console URL, and run ID so both paths produce identical bodies.orchestrator/internal/kanban/poller_test.go (1)
87-109: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe default board hides the column-drift branch from all tests.
When
refis not inf.boards, this fake returns a synthetic board that always containsai,agent, anddone. Every test uses trigger columnaiand done columndone, so the column check invalidatePluginKanbanBoardatorchestrator/internal/kanban/poller.goLines 231-236 always passes.
TestPluginKanbanAutomationStopsAtBoardDriftAndRecoversonly exercisesconfigBoardMissing, which is theErrDocNotFoundpath. No test covers a board that exists but has lost the configured trigger or done column. A regression in that branch would not fail CI.Add a case that seeds
api.boards["b"]with a board missing the done column, and assert theboard_drift:error and that no Run is dispatched.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/kanban/poller_test.go` around lines 87 - 109, Extend the poller tests with a board-drift case that seeds fakeAPI.boards["b"] with a board containing the configured trigger column but no done column. Invoke the plugin Kanban automation flow, assert it returns the expected board_drift: error, and verify the fake API recorded no Run dispatch. Keep the existing configBoardMissing recovery test unchanged.console/src/api/mockClient.ts (2)
2192-2193: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse the persisted column labels in the policy mock.
The orchestrator returns a human-readable label resolved from the board, and
ProjectAutomationSpec.kanbannow carriestrigger_labelanddone_label. This mock returns the raw column key as the label, so demo mode rendersaiwhere production rendersAgent queue.♻️ Proposed fix to use the persisted labels
- trigger_column: { key: spec.kanban.trigger_column, label: spec.kanban.trigger_column }, - done_column: { key: spec.kanban.done_column, label: spec.kanban.done_column }, + trigger_column: { + key: spec.kanban.trigger_column, + label: spec.kanban.trigger_label ?? spec.kanban.trigger_column, + }, + done_column: { + key: spec.kanban.done_column, + label: spec.kanban.done_label ?? spec.kanban.done_column, + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/src/api/mockClient.ts` around lines 2192 - 2193, The policy mock’s kanban column labels currently use raw keys instead of persisted labels. Update the policy mock mapping to use spec.kanban.trigger_label and spec.kanban.done_label for the label fields, while keeping trigger_column and done_column keys sourced from their existing column properties.
2202-2208: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider seeding one execution so the demo exercises the history UI.
This mock always returns an empty page. The execution list, the pagination control, the blocked and terminal badges, and the locale keys added in this PR are therefore unreachable in demo mode and in any offline test that uses
createMockClient. One seeded terminal execution plus one blocked execution would cover the states without fabricating a whole playback engine.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/src/api/mockClient.ts` around lines 2202 - 2208, Update listServiceKanbanCardExecutions in the mock client to return seeded execution records instead of an empty items array, including at least one terminal execution and one blocked execution with fields required by the history UI. Provide enough records and a non-null next_cursor to exercise pagination, while preserving the existing KanbanCardExecutionsPage shape and delay wrapper.orchestrator/internal/api/service_kanban.go (1)
302-316: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBound the per-page
GetRunfan-out or batch it.The loop performs one
GetRuncall per occurrence. Withlimit=50this is 50 sequential store round trips per request. The count is bounded, so this is not a stability risk, but a batched lookup by run ID would remove the per-item latency on the request thread.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@orchestrator/internal/api/service_kanban.go` around lines 302 - 316, Update the occurrence-loading flow around serviceKanbanExecutionView to avoid one sequential GetRun call per occurrence: collect the page’s non-empty, unique RunID values and use an available batch lookup, or otherwise bound/concurrently execute the lookups with the existing page limit. Preserve not-found handling by passing nil for missing runs and continue returning an internal error for other store failures.console/src/api/queries.ts (1)
793-799: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLimit the polling check to the current occurrence.
refetchIntervalscans every loaded page, including earlier history pages fetched by "Load earlier executions". An older occurrence that staysblockedkeeps the 5-second poll active for as long as the Card detail stays open, even after the current occurrence reachesterminal. Restrict the check to the newest occurrence, which is the only item that can still advance.♻️ Proposed change to poll only on the current occurrence
refetchInterval: (query) => { - const pages = query.state.data?.pages; - return pages?.some((page) => page.items.some((item) => - item.status === 'received' || item.status === 'blocked' || item.status === 'queued' || item.status === 'running')) + const current = query.state.data?.pages[0]?.items[0]; + return current && (current.status === 'received' || current.status === 'blocked' + || current.status === 'queued' || current.status === 'running') ? 5_000 : false; },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/src/api/queries.ts` around lines 793 - 799, Update the refetchInterval callback to inspect only the newest/current occurrence rather than scanning every item across query.state.data.pages. Preserve the existing 5-second interval when that current item is received, blocked, queued, or running, and return false once it reaches a terminal state.design/kanban-agent-executions.html (1)
264-264: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHide the decorative receipt icons from assistive technology.
The receipt icons on lines 264 and 278 carry no
aria-hidden, and line 291 renders a bare!character. Each receipt already states its status in the adjacent<strong>text. A screen reader announces these glyphs as extra unlabeled content. Line 119 already usesaria-hidden="true"for the same purpose; follow that pattern.Also applies to: 278-278, 291-291
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@design/kanban-agent-executions.html` at line 264, Mark the decorative receipt icons at the spans around the receipt entries, including the SVG/icon at the locations corresponding to lines 264 and 278, with aria-hidden="true", and hide the standalone “!” character at the corresponding line 291 from assistive technology as well. Follow the existing aria-hidden pattern used near line 119 while preserving the adjacent strong status text.console/src/pages/KanbanBoardModal.test.tsx (1)
183-237: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the run link and the writeback states.
The three new tests cover blocked, load failure, and deleted-Card history. No test covers an occurrence that carries
run, so theOpen Runlink target stays unverified, and no test coversreceipt.writeback === 'pending'or'unavailable'. Both paths are user-visible and both are cheap to assert with the existing fake client.Also applies to: 239-253, 255-309
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@console/src/pages/KanbanBoardModal.test.tsx` around lines 183 - 237, Add tests alongside the existing KanbanBoardModal execution-history tests for occurrences with a non-null run, asserting the user-visible “Open Run” link and its target. Add coverage for receipt.writeback values “pending” and “unavailable”, asserting each corresponding displayed state while preserving the existing blocked, load-failure, and deleted-card scenarios.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/src/i18n/locales/en.ts`:
- Line 1066: Update the noExecutions translation in
console/src/i18n/locales/en.ts (1066-1066), console/src/i18n/locales/ja.ts
(731-731), and console/src/i18n/locales/ko.ts (731-731) to refer to the
configured trigger column rather than a literal “Starts jcode”/「jcode
を開始」/“jcode 시작” column name. Update the corresponding English assertion in
console/src/pages/KanbanBoardModal.test.tsx (line 251) to match the revised
wording.
In `@console/src/i18n/locales/zh-Hant.ts`:
- Line 708: Update the zh-Hant `policyBlocked` translation to use the
established `受阻` term, matching `run.statusName.blocked`,
`components.statusBadge.blocked`, and `executionState.blocked` for consistent
blocked-state terminology.
In `@console/src/pages/KanbanBoardModal.module.css`:
- Around line 22-34: Update the .policyStrip grid definition so its columns can
wrap responsively instead of requiring all five minimum widths simultaneously,
using an auto-fit/auto-fill layout with the existing minimum column size.
Preserve the current single-column behavior from the max-width: 700px media
query while preventing horizontal overflow at larger font sizes.
In `@console/src/pages/KanbanBoardModal.tsx`:
- Line 259: Update the timestamp rendering around the time elements in
KanbanBoardModal to format dates using the selected i18next language rather than
the browser default; pass i18n.language to toLocaleString or reuse an existing
shared formatter identified by the repository search, preserving the current
timestamp values and display locations.
- Around line 576-582: Update the renderCardSupplement callback in
KanbanBoardModal so CardExecutionsSupplement receives card.relativePath as
documentPath instead of card.id. If the receipt API also requires the Card
identifier, pass card.id through the appropriate separate document_id prop
without changing the existing serviceId or workspaceId values.
In `@design/assets/kanban-agent-executions.css`:
- Line 324: Remove the redundant color declaration from .ka-receipt-icon; its
color already inherits from .ka-receipt, avoiding the stylelint
value-keyword-case error.
In `@design/kanban-agent-executions.html`:
- Line 202: Update the dialog elements using class ka-card-dialog so
aria-labelledby references a visually hidden static heading rather than the
cardTitle input, and remove aria-describedby or point it to short static
descriptive text instead of the cardDescription textarea. Apply the same
accessibility fix to all corresponding dialog instances.
In `@orchestrator/internal/api/service_kanban_test.go`:
- Around line 229-235: Add an HTTP status assertion immediately after the
drift-policy request in the kanban test and before calling decode or accessing
driftPolicy["health"]. Keep the existing response decoding and health assertions
unchanged.
In `@orchestrator/internal/api/service_kanban.go`:
- Around line 188-200: The model configuration response at
orchestrator/internal/api/service_kanban.go:188-200 must stop returning the
English “Not configured” label: set the label empty and rely on the existing
model_not_configured blocker code for Console localization. At
orchestrator/internal/api/service_kanban.go:431-448, remove the English summary
strings or explicitly mark summary diagnostic-only and ensure Console
presentation uses status, outcome, and reason_code with
kanban.executionSummary.* localization keys.
In `@orchestrator/internal/jtype/client.go`:
- Around line 378-390: Update the board scan around GetBoardByDoc in the
document iteration to skip parse failures and continue checking subsequent
.board documents instead of returning immediately. Preserve successful configID
matching, and return ErrDocNotFound only after all candidates are examined; if
transport errors must remain visible, retain and return the last transport error
when no board matches.
In `@orchestrator/internal/kanban/poller_test.go`:
- Around line 813-815: Initialize the log field with testLogger(t) in both
directly constructed Poller values, including the constructions used by
dispatchPluginKanbanOccurrence, blockPluginKanbanOccurrence,
projectPluginKanbanReceipt, and pluginKanbanRepositoryBlocker tests, while
preserving their existing st, models, and now fields.
In `@orchestrator/internal/kanban/poller.go`:
- Around line 279-281: Store failures in Poller must become fail-visible: in
orchestrator/internal/kanban/poller.go:279-281, set a bootstrap_unavailable
error before returning from ObservePluginKanbanCard failure; at 289-291 log
AdvancePluginKanbanTrigger errors with advanced; at 350-352 and 363-365 log
MarkPluginKanbanCardUnavailable and ObservePluginKanbanCard failures; at 373-375
log both advanceErr and advanced; at 406-409 set service_unavailable for
GetService failure; at 473-475 log CreatePluginKanbanOccurrenceRun err and
attached separately; and at 549-551, 559-561, and 586-588 log failures from
ListPluginKanbanDispatchableOccurrences, ListPluginKanbanReceiptPending, and
SetPluginKanbanOccurrenceReceiptPhase, including the automation ID where
appropriate.
In `@orchestrator/internal/reconciler/reconciler.go`:
- Around line 1077-1081: Update the writeback error paths around ListComments,
AddComment, and MoveCard so failures other than not-found are logged and
recorded on the occurrence, using the existing MarkPluginKanbanOccurrenceReceipt
flow or equivalent durable state update. Preserve the unavailable-plugin
handling for isJTypeNotFound errors, and check and log the returned error from
MarkPluginKanbanWriteback so store failures are not discarded.
In `@orchestrator/internal/store/memory.go`:
- Line 2796: Update the claim-retention predicate in the memory-store deletion
logic to match PGStore.DeletePluginAutomation: retain claims for the matching
automation and unassociated run when any occurrence exists for the same
automation and document, rather than requiring LatestOccurrenceID to be
populated. Preserve claims when occurrence rows exist even if LatestOccurrenceID
is empty.
- Around line 3271-3290: Update MemStore.SetPluginKanbanClaimRun to also update
the occurrence referenced by claim.LatestOccurrenceID when it already exists:
assign the new RunID and transition its State to domain.KanbanOccurrenceQueued.
Match PGStore.SetPluginKanbanClaimRun by returning ErrAlreadyExists when that
occurrence already has a run, while preserving legacy occurrence creation for
claims without a pointer.
In
`@orchestrator/internal/store/migrations/0058_kanban_execution_occurrences.sql`:
- Around line 97-103: Make the backfill identified by the UPDATE on
automation_kanban_claims deterministic by selecting only the newest matching
automation_kanban_occurrences row for each claim’s automation_id and
document_id, using an explicit ordering such as occurrence creation/update time
with a stable tie-breaker. Preserve the existing latest_occurrence_id and
updated_at assignments while ensuring reruns always point to the newest
occurrence.
In `@orchestrator/internal/store/plugin_kanban_occurrence.go`:
- Around line 345-366: Update MemStore.MarkPluginKanbanOccurrenceReceipt to
match PGStore semantics: preserve the writebackError argument even when
writtenAt is provided, set UpdatedAt using the current UTC time rather than
writtenAt, and return ErrConflict for missing occurrences as well as phase
mismatches.
- Around line 216-222: Guard the claim lookup in the in-memory update path
before modifying or writing it back: after computing the key with
pluginKanbanClaimKey, verify the claim exists in m.pluginKanbanClaims and return
or otherwise skip the update when absent. Only assign RunID, WritebackAt,
LatestOccurrenceID, and UpdatedAt and write back the claim when an existing
claim is found, preserving its identity fields.
In `@orchestrator/internal/store/plugins.go`:
- Around line 815-826: Track whether the occurrence UPDATE succeeds in both
MarkPluginKanbanWriteback and MarkPluginKanbanWritebackUnavailable at
orchestrator/internal/store/plugins.go:815-826 and 867-879. When the subsequent
claim UPDATE affects zero rows, return (false, nil) if the occurrence update was
skipped; retain the existing hard error when it was applied.
---
Nitpick comments:
In `@console/src/api/mockClient.ts`:
- Around line 2192-2193: The policy mock’s kanban column labels currently use
raw keys instead of persisted labels. Update the policy mock mapping to use
spec.kanban.trigger_label and spec.kanban.done_label for the label fields, while
keeping trigger_column and done_column keys sourced from their existing column
properties.
- Around line 2202-2208: Update listServiceKanbanCardExecutions in the mock
client to return seeded execution records instead of an empty items array,
including at least one terminal execution and one blocked execution with fields
required by the history UI. Provide enough records and a non-null next_cursor to
exercise pagination, while preserving the existing KanbanCardExecutionsPage
shape and delay wrapper.
In `@console/src/api/queries.ts`:
- Around line 793-799: Update the refetchInterval callback to inspect only the
newest/current occurrence rather than scanning every item across
query.state.data.pages. Preserve the existing 5-second interval when that
current item is received, blocked, queued, or running, and return false once it
reaches a terminal state.
In `@console/src/pages/KanbanBoardModal.test.tsx`:
- Around line 183-237: Add tests alongside the existing KanbanBoardModal
execution-history tests for occurrences with a non-null run, asserting the
user-visible “Open Run” link and its target. Add coverage for receipt.writeback
values “pending” and “unavailable”, asserting each corresponding displayed state
while preserving the existing blocked, load-failure, and deleted-card scenarios.
In `@design/assets/kanban-agent-executions.js`:
- Around line 37-40: Update the initialization flow around the button click
listeners to derive the initial state from the button marked pressed in the
static markup, then call setState once on load before user interaction. Preserve
the existing click handlers while ensuring aria-pressed, panel visibility,
policy tone, and policy text are synchronized immediately.
In `@design/kanban-agent-executions.html`:
- Line 264: Mark the decorative receipt icons at the spans around the receipt
entries, including the SVG/icon at the locations corresponding to lines 264 and
278, with aria-hidden="true", and hide the standalone “!” character at the
corresponding line 291 from assistive technology as well. Follow the existing
aria-hidden pattern used near line 119 while preserving the adjacent strong
status text.
In `@orchestrator/internal/api/service_kanban.go`:
- Around line 302-316: Update the occurrence-loading flow around
serviceKanbanExecutionView to avoid one sequential GetRun call per occurrence:
collect the page’s non-empty, unique RunID values and use an available batch
lookup, or otherwise bound/concurrently execute the lookups with the existing
page limit. Preserve not-found handling by passing nil for missing runs and
continue returning an internal error for other store failures.
In `@orchestrator/internal/domain/plugins.go`:
- Around line 341-377: Define typed enums and named constants for Outcome,
ReceiptPhase, and WritebackState alongside KanbanOccurrenceState, covering the
existing values used by MemStore and PGStore such as pending, complete,
not_required, accepted, and terminal. Update PluginKanbanOccurrence and all
store assignments/comparisons to use these types and shared constants instead of
raw strings.
In `@orchestrator/internal/jtype/board_resolve_test.go`:
- Around line 129-149: Expand GetBoardByConfigID test coverage with cases for an
unknown configID returning ErrDocNotFound and an empty configID returning
ErrDocNotFound without making an HTTP request. Also add a workspace listing
containing a malformed .board document before the valid configured document, and
assert the valid board still resolves rather than aborting on the malformed
content.
In `@orchestrator/internal/jtype/client.go`:
- Around line 369-377: The GetBoardByConfigID path currently lists and fetches
every board document for each validation call; add poller-level caching from
configID to document ID around validatePluginKanbanBoard, reuse cached mappings
across ticks, and re-resolve through GetBoardByConfigID only when the cached
document is missing or no longer matches the configured ID.
In `@orchestrator/internal/kanban/poller_test.go`:
- Around line 87-109: Extend the poller tests with a board-drift case that seeds
fakeAPI.boards["b"] with a board containing the configured trigger column but no
done column. Invoke the plugin Kanban automation flow, assert it returns the
expected board_drift: error, and verify the fake API recorded no Run dispatch.
Keep the existing configBoardMissing recovery test unchanged.
In `@orchestrator/internal/kanban/poller.go`:
- Around line 297-316: Cap the pagination loop in pluginAutomationEventHead so a
single tick cannot walk an unbounded event history. Track iterations or pages,
stop at a reasonable configured limit, and return the highest observed head when
the cap is reached; preserve the existing backwards-cursor and
non-advancing-feed error guards.
In `@orchestrator/internal/reconciler/kanban_test.go`:
- Around line 290-297: Update TestPluginKanbanWritebackUsesInstallationWorkspace
to seed the deleted-card occurrence through ObservePluginKanbanCard and
CreatePluginKanbanOccurrenceRun, matching the setup used by the sibling test
around the existing occurrence API. Replace the EnsurePluginKanbanClaim and
SetPluginKanbanClaimRun calls so the test includes the production-shaped
occurrence and EntryColumn history.
In `@orchestrator/internal/reconciler/reconciler.go`:
- Around line 1096-1110: Extract the accepted-receipt body construction into one
shared builder located next to jtype.KanbanReceiptMarker, including the poller’s
unconditional Run link behavior. Replace the local construction in the
reconciler block and the duplicate logic in poller.go with calls to that shared
builder, passing the service name, model, console URL, and run ID so both paths
produce identical bodies.
In `@orchestrator/internal/store/memory.go`:
- Around line 3307-3323: Remove the redundant claim.LatestOccurrenceID != ""
guard in the occurrence lookup within the affected writeback methods, matching
MarkPluginKanbanWritebackUnavailable’s direct map access pattern. Preserve the
existing claim.LatestOccurrenceID == occurrenceID equality guard and pending
WritebackState check, including the corresponding logic in the additional
affected ranges.
In `@orchestrator/internal/store/plugin_kanban_occurrence_test.go`:
- Around line 288-341: Extend the plugin Kanban occurrence tests with cases for
SetPluginKanbanClaimRun on a claim with LatestOccurrenceID already set,
asserting the occurrence becomes queued and attaches the Run, and for repeated
MarkPluginKanbanWriteback calls with identical arguments, asserting the second
returns false with no error, including an empty occurrenceID where applicable.
Update the existing not-found assertion near the referenced comparison to use
errors.Is with ErrNotFound.
In `@orchestrator/internal/store/plugin_kanban_occurrence.go`:
- Around line 563-566: Replace the recursive ObservePluginKanbanCard calls in
both unique-violation paths with a bounded retry loop, allowing at most two
transaction attempts. Preserve rollback and retry behavior for the first
conflict, but return the final error when the retry limit is reached; update the
surrounding ObservePluginKanbanCard flow consistently.
In `@orchestrator/internal/store/plugins.go`:
- Around line 738-780: Update ListPluginKanbanRunsAwaitingWriteback to select
the required run and claim columns alongside qualifiedPluginKanbanOccurrenceCols
in its existing joined query, then scan all three records in the rows.Next loop
using the established scan helpers. Remove the per-occurrence GetRun and
scanPluginKanbanClaim queries, while preserving the existing error wrapping and
PluginKanbanWriteback assembly.
In `@orchestrator/internal/store/store.go`:
- Around line 549-565: Add interface comments documenting the idempotency and
error contracts for the new PluginKanban methods, using existing comments such
as MarkKanbanWriteback as the style reference. Explicitly record
CreatePluginKanbanOccurrenceRun’s existing-run result, stale and
legacy-selection behavior for both writeback methods, ErrConflict conditions for
receipt and blocked updates, and AdvancePluginKanbanTrigger’s compare-and-set
and one-time bootstrap semantics; ensure the comments cover all 17 new methods
without changing implementation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 61168e9a-e7e4-4853-bcfa-f6b97ba124c3
⛔ Files ignored due to path filters (1)
console/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
console/package.jsonconsole/src/api/client.test.tsconsole/src/api/client.tsconsole/src/api/mockClient.tsconsole/src/api/queries.tsconsole/src/api/types.tsconsole/src/i18n/locales/en.tsconsole/src/i18n/locales/ja.tsconsole/src/i18n/locales/ko.tsconsole/src/i18n/locales/zh-Hans.tsconsole/src/i18n/locales/zh-Hant.tsconsole/src/pages/KanbanBoardModal.module.cssconsole/src/pages/KanbanBoardModal.test.tsxconsole/src/pages/KanbanBoardModal.tsxdesign/README.mddesign/assets/kanban-agent-executions.cssdesign/assets/kanban-agent-executions.jsdesign/kanban-agent-executions.htmldocs/23-kanban-execution-receipts.mddocs/README.mdorchestrator/internal/api/api.goorchestrator/internal/api/service_kanban.goorchestrator/internal/api/service_kanban_test.goorchestrator/internal/domain/plugins.goorchestrator/internal/jtype/board_resolve_test.goorchestrator/internal/jtype/client.goorchestrator/internal/jtype/client_test.goorchestrator/internal/kanban/poller.goorchestrator/internal/kanban/poller_test.goorchestrator/internal/reconciler/kanban_test.goorchestrator/internal/reconciler/reconciler.goorchestrator/internal/store/memory.goorchestrator/internal/store/migrations/0058_kanban_execution_occurrences.sqlorchestrator/internal/store/migrations/0059_kanban_column_labels.sqlorchestrator/internal/store/plugin_kanban_occurrence.goorchestrator/internal/store/plugin_kanban_occurrence_test.goorchestrator/internal/store/plugins.goorchestrator/internal/store/plugins_test.goorchestrator/internal/store/store.go
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/README.md
| executionsTitle: 'Cloud executions', | ||
| loadingExecutions: 'Loading Cloud executions…', | ||
| executionsUnavailable: 'Cloud executions could not be loaded.', | ||
| noExecutions: 'No Cloud execution yet. Move this Card into the Starts jcode column to request one.', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
noExecutions names a column that does not exist. All three locales tell the user to move the Card into a column called "Starts jcode". That is the wording of the policyTrigger sentence, not a board column name. The real column name is configured per Service and is interpolated elsewhere through {column}.
console/src/i18n/locales/en.ts#L1066-L1066: reword to refer to the configured trigger column, for example "Move this Card into the trigger column to request one."console/src/i18n/locales/ja.ts#L731-L731: apply the same rewording, and remove the literal 「jcode を開始」 column name.console/src/i18n/locales/ko.ts#L731-L731: apply the same rewording, and remove the literal “jcode 시작” column name.
console/src/pages/KanbanBoardModal.test.tsx line 251 asserts the English literal, so update that assertion together with the string.
📍 Affects 3 files
console/src/i18n/locales/en.ts#L1066-L1066(this comment)console/src/i18n/locales/ja.ts#L731-L731console/src/i18n/locales/ko.ts#L731-L731
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/src/i18n/locales/en.ts` at line 1066, Update the noExecutions
translation in console/src/i18n/locales/en.ts (1066-1066),
console/src/i18n/locales/ja.ts (731-731), and console/src/i18n/locales/ko.ts
(731-731) to refer to the configured trigger column rather than a literal
“Starts jcode”/「jcode を開始」/“jcode 시작” column name. Update the corresponding
English assertion in console/src/pages/KanbanBoardModal.test.tsx (line 251) to
match the revised wording.
| policyWriteback: '成功後留言並移動到 {column}', | ||
| policyCommentOnly: '結果寫入 Card 留言', | ||
| policyReady: '可執行', | ||
| policyBlocked: '已阻塞 · {blocker}', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the established zh-Hant term for the blocked state.
This entry uses 已阻塞. The existing zh-Hant translations for the same concept use 受阻: run.statusName.blocked at line 79, components.statusBadge.blocked at line 604, and executionState.blocked at line 734 in this same block. Use one term for one concept.
🌐 Proposed fix for the terminology inconsistency
- policyBlocked: '已阻塞 · {blocker}',
+ policyBlocked: '受阻 · {blocker}',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| policyBlocked: '已阻塞 · {blocker}', | |
| policyBlocked: '受阻 · {blocker}', |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/src/i18n/locales/zh-Hant.ts` at line 708, Update the zh-Hant
`policyBlocked` translation to use the established `受阻` term, matching
`run.statusName.blocked`, `components.statusBadge.blocked`, and
`executionState.blocked` for consistent blocked-state terminology.
| .policyStrip { | ||
| display: grid; | ||
| grid-template-columns: minmax(10rem, 1.3fr) repeat(4, minmax(8rem, 1fr)); | ||
| align-items: center; | ||
| gap: var(--space-3); | ||
| padding: var(--space-3) var(--space-4); | ||
| border: 1px solid var(--color-border); | ||
| border-radius: var(--radius-lg); | ||
| background: var(--color-panel-raised); | ||
| color: var(--color-text-muted); | ||
| font-size: var(--fs-caption); | ||
| line-height: var(--lh-normal); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The policy strip overflows between the column minimums and the breakpoint.
.policyStrip sets five columns with minimums of 10rem plus 4 × 8rem, which is 42rem before gaps and padding. The collapse to one column happens at max-width: 700px, a fixed pixel value.
At the default root font size the strip needs about 46rem, so it already overflows horizontally just above 700px. A user who raises the base font size makes this worse: at 150% text size the strip needs about 63rem while the media query still waits for 700px. The result is horizontal scrolling in the modal, which fails reflow expectations.
Tie the breakpoint to the same unit as the minimums, or let the grid collapse on its own.
♿ Proposed fix
.policyStrip {
display: grid;
- grid-template-columns: minmax(10rem, 1.3fr) repeat(4, minmax(8rem, 1fr));
+ grid-template-columns: minmax(10rem, 1.3fr) repeat(auto-fit, minmax(8rem, 1fr));
align-items: center;auto-fit with the same minimum wraps the trailing columns instead of overflowing, and the existing max-width: 700px rule still forces the single-column layout on small screens.
Also applies to: 285-287
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/src/pages/KanbanBoardModal.module.css` around lines 22 - 34, Update
the .policyStrip grid definition so its columns can wrap responsively instead of
requiring all five minimum widths simultaneously, using an auto-fit/auto-fill
layout with the existing minimum column size. Preserve the current single-column
behavior from the max-width: 700px media query while preventing horizontal
overflow at larger font sizes.
| > | ||
| <div className={styles.executionHeading}> | ||
| <strong>{executionStateLabel(current, t)}</strong> | ||
| <time dateTime={current.updated_at}>{new Date(current.updated_at).toLocaleString()}</time> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Format timestamps with the selected app language.
new Date(...).toLocaleString() uses the browser locale. The console selects its language through i18next and ships en, ja, ko, zh-Hans, and zh-Hant. A user who selects Japanese in the console can still see English-formatted timestamps. Pass i18n.language to toLocaleString, or reuse the existing shared time formatter if one exists.
Run the following script to check for an existing formatter to reuse:
#!/bin/bash
# Description: Find existing date/time formatting helpers in the console.
rg -nP --type=ts --type=tsx -C3 'toLocaleString|toLocaleDateString|Intl\.DateTimeFormat|formatRelative|formatTime' console/src | head -80
fd -t f . console/src/i18n --exec cat -n {}Also applies to: 280-280
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/src/pages/KanbanBoardModal.tsx` at line 259, Update the timestamp
rendering around the time elements in KanbanBoardModal to format dates using the
selected i18next language rather than the browser default; pass i18n.language to
toLocaleString or reuse an existing shared formatter identified by the
repository search, preserving the current timestamp values and display
locations.
| renderCardSupplement={serviceId ? (card) => ( | ||
| <CardExecutionsSupplement | ||
| serviceId={serviceId} | ||
| workspaceId={link.workspace_id} | ||
| documentPath={card.id} | ||
| /> | ||
| ) : undefined} |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the renderCardSupplement card type in jtype-board-react and the server documentPath contract.
fd -t f -e d.ts . --full-path '*jtype-board-react*' --exec rg -n -C5 'renderCardSupplement|interface .*Card\b' {}
rg -nP -C5 'documentPath|document_path' --glob '*.go' | head -60Repository: cnjack/cloud
Length of output: 257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package references to jtype-board-react =="
git ls-files | rg '(^|/)(package\.json|pnpm-lock\.yaml|package-lock\.json|yarn\.lock)$|jtype-board-react' || true
echo
echo "== KanbanBoardModal context =="
if [ -f console/src/pages/KanbanBoardModal.tsx ]; then
wc -l console/src/pages/KanbanBoardModal.tsx
sed -n '530,605p' console/src/pages/KanbanBoardModal.tsx
fi
echo
echo "== documentPath references =="
rg -n -C4 'documentPath|document_path|CardExecutionsSupplement' --glob '!node_modules' . | head -120
echo
echo "== board package files with card types/customers =="
git ls-files | rg 'jtype|board|kanban|execute|execution' || trueRepository: cnjack/cloud
Length of output: 16795
Pass the Card document path into CardExecutionsSupplement.
documentPath is the workspace-relative .card path, but card.id is a Card identifier in jtype-board-react; using it here makes every receipt query target the wrong card. Use card.relativePath for the supplement, and include any needed document_id separately if the API requires it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/src/pages/KanbanBoardModal.tsx` around lines 576 - 582, Update the
renderCardSupplement callback in KanbanBoardModal so CardExecutionsSupplement
receives card.relativePath as documentPath instead of card.id. If the receipt
API also requires the Card identifier, pass card.id through the appropriate
separate document_id prop without changing the existing serviceId or workspaceId
values.
| if claim.LatestOccurrenceID == "" { | ||
| automation, automationOK := m.pluginAutomations[automationID] | ||
| trigger, triggerOK := m.pluginKanbanTriggers[automationID] | ||
| if !automationOK || !triggerOK { | ||
| return ErrNotFound | ||
| } | ||
| now := time.Now().UTC() | ||
| occurrence := domain.PluginKanbanOccurrence{ | ||
| ID: domain.NewID(), AutomationID: automationID, ServiceID: automation.ServiceID, | ||
| InstallationID: claim.InstallationID, WorkspaceID: claim.WorkspaceID, | ||
| DocumentID: documentID, DocumentPath: claim.DocumentPath, DoneColumn: claim.DoneColumn, | ||
| EventKey: "legacy:" + documentID + ":" + runID, EntryColumn: trigger.TriggerColumn, | ||
| State: domain.KanbanOccurrenceQueued, RunID: runID, WritebackState: "pending", | ||
| CreatedAt: now, UpdatedAt: now, | ||
| } | ||
| m.pluginKanbanOccurrences[occurrence.ID] = occurrence | ||
| claim.LatestOccurrenceID = occurrence.ID | ||
| } | ||
| claim.RunID = runID | ||
| claim.UpdatedAt = time.Now().UTC() |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
MemStore.SetPluginKanbanClaimRun does not attach the Run to an existing occurrence.
The memory path creates a legacy occurrence only when claim.LatestOccurrenceID == "". When the pointer is already set, it writes claim.RunID and leaves the occurrence at RunID == "" and State == received.
PGStore.SetPluginKanbanClaimRun (orchestrator/internal/store/plugins.go lines 706-714) instead updates that occurrence to run_id=$2,state='queued' and returns ErrAlreadyExists when the occurrence already owns a run.
The divergence is observable. ListPluginKanbanRunsAwaitingWriteback still selects the claim, while the Card executions API derives status from the occurrence; orchestrator/internal/api/service_kanban_test.go lines 296-307 shows a queued occurrence with an unresolvable Run rendering as blocked / run_unavailable. Under MemStore, a healthy Run can therefore surface as blocked, and every memory-backed test of this path validates the wrong contract.
🐛 Proposed parity fix
if claim.LatestOccurrenceID == "" {
automation, automationOK := m.pluginAutomations[automationID]
trigger, triggerOK := m.pluginKanbanTriggers[automationID]
if !automationOK || !triggerOK {
return ErrNotFound
}
now := time.Now().UTC()
occurrence := domain.PluginKanbanOccurrence{
ID: domain.NewID(), AutomationID: automationID, ServiceID: automation.ServiceID,
InstallationID: claim.InstallationID, WorkspaceID: claim.WorkspaceID,
DocumentID: documentID, DocumentPath: claim.DocumentPath, DoneColumn: claim.DoneColumn,
EventKey: "legacy:" + documentID + ":" + runID, EntryColumn: trigger.TriggerColumn,
State: domain.KanbanOccurrenceQueued, RunID: runID, WritebackState: "pending",
CreatedAt: now, UpdatedAt: now,
}
m.pluginKanbanOccurrences[occurrence.ID] = occurrence
claim.LatestOccurrenceID = occurrence.ID
+ } else {
+ occurrence, exists := m.pluginKanbanOccurrences[claim.LatestOccurrenceID]
+ if !exists || occurrence.RunID != "" {
+ return ErrAlreadyExists
+ }
+ occurrence.RunID = runID
+ occurrence.State = domain.KanbanOccurrenceQueued
+ occurrence.UpdatedAt = time.Now().UTC()
+ m.pluginKanbanOccurrences[claim.LatestOccurrenceID] = occurrence
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestrator/internal/store/memory.go` around lines 3271 - 3290, Update
MemStore.SetPluginKanbanClaimRun to also update the occurrence referenced by
claim.LatestOccurrenceID when it already exists: assign the new RunID and
transition its State to domain.KanbanOccurrenceQueued. Match
PGStore.SetPluginKanbanClaimRun by returning ErrAlreadyExists when that
occurrence already has a run, while preserving legacy occurrence creation for
claims without a pointer.
| UPDATE automation_kanban_claims c | ||
| SET latest_occurrence_id = o.id, | ||
| updated_at = GREATEST(c.updated_at, o.updated_at) | ||
| FROM automation_kanban_occurrences o | ||
| WHERE o.automation_id = c.automation_id | ||
| AND o.document_id = c.document_id | ||
| AND c.latest_occurrence_id = ''; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Make the claim pointer backfill deterministic.
UPDATE ... FROM joins occurrences on (automation_id, document_id) only. On the first run each claim has at most one legacy occurrence, so the result is stable. On a re-run, application-created occurrences can share the same key. PostgreSQL then uses one arbitrary matching row, so latest_occurrence_id can point at an older occurrence. classifyPluginKanbanEntry and ListPluginKanbanRunsAwaitingWriteback both key off that pointer, so a stale pointer suppresses a legitimate new occurrence or routes writeback to the wrong row.
Select the newest occurrence explicitly.
🛠️ Proposed deterministic backfill
UPDATE automation_kanban_claims c
-SET latest_occurrence_id = o.id,
- updated_at = GREATEST(c.updated_at, o.updated_at)
-FROM automation_kanban_occurrences o
-WHERE o.automation_id = c.automation_id
- AND o.document_id = c.document_id
- AND c.latest_occurrence_id = '';
+SET latest_occurrence_id = o.id,
+ updated_at = GREATEST(c.updated_at, o.updated_at)
+FROM (
+ SELECT DISTINCT ON (automation_id, document_id)
+ automation_id, document_id, id, updated_at
+ FROM automation_kanban_occurrences
+ ORDER BY automation_id, document_id, created_at DESC, id DESC
+) o
+WHERE o.automation_id = c.automation_id
+ AND o.document_id = c.document_id
+ AND c.latest_occurrence_id = '';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| UPDATE automation_kanban_claims c | |
| SET latest_occurrence_id = o.id, | |
| updated_at = GREATEST(c.updated_at, o.updated_at) | |
| FROM automation_kanban_occurrences o | |
| WHERE o.automation_id = c.automation_id | |
| AND o.document_id = c.document_id | |
| AND c.latest_occurrence_id = ''; | |
| UPDATE automation_kanban_claims c | |
| SET latest_occurrence_id = o.id, | |
| updated_at = GREATEST(c.updated_at, o.updated_at) | |
| FROM ( | |
| SELECT DISTINCT ON (automation_id, document_id) | |
| automation_id, document_id, id, updated_at | |
| FROM automation_kanban_occurrences | |
| ORDER BY automation_id, document_id, created_at DESC, id DESC | |
| ) o | |
| WHERE o.automation_id = c.automation_id | |
| AND o.document_id = c.document_id | |
| AND c.latest_occurrence_id = ''; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestrator/internal/store/migrations/0058_kanban_execution_occurrences.sql`
around lines 97 - 103, Make the backfill identified by the UPDATE on
automation_kanban_claims deterministic by selecting only the newest matching
automation_kanban_occurrences row for each claim’s automation_id and
document_id, using an explicit ordering such as occurrence creation/update time
with a stable tie-breaker. Preserve the existing latest_occurrence_id and
updated_at assignments while ensuring reruns always point to the newest
occurrence.
| key := pluginKanbanClaimKey(occurrence.AutomationID, occurrence.DocumentID) | ||
| claim := m.pluginKanbanClaims[key] | ||
| claim.RunID = run.ID | ||
| claim.WritebackAt = nil | ||
| claim.LatestOccurrenceID = occurrenceID | ||
| claim.UpdatedAt = now | ||
| m.pluginKanbanClaims[key] = claim |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Guard the claim read before writing it back.
Line 217 reads the claim with a map index and no existence check. If the key is absent, the map receives a zero-valued claim that carries only RunID, LatestOccurrenceID, and UpdatedAt. AutomationID, DocumentID, WorkspaceID, and DocumentPath stay empty, so GetPluginKanbanClaimByPath can never resolve it and ListPluginKanbanRunsAwaitingWriteback reports a claim with no document identity. The PostgreSQL path cannot create such a row, because its UPDATE matches on (automation_id, document_id).
🛡️ Proposed guard
key := pluginKanbanClaimKey(occurrence.AutomationID, occurrence.DocumentID)
- claim := m.pluginKanbanClaims[key]
- claim.RunID = run.ID
- claim.WritebackAt = nil
- claim.LatestOccurrenceID = occurrenceID
- claim.UpdatedAt = now
- m.pluginKanbanClaims[key] = claim
+ if claim, exists := m.pluginKanbanClaims[key]; exists {
+ claim.RunID = run.ID
+ claim.WritebackAt = nil
+ claim.LatestOccurrenceID = occurrenceID
+ claim.UpdatedAt = now
+ m.pluginKanbanClaims[key] = claim
+ }
return true, nil📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| key := pluginKanbanClaimKey(occurrence.AutomationID, occurrence.DocumentID) | |
| claim := m.pluginKanbanClaims[key] | |
| claim.RunID = run.ID | |
| claim.WritebackAt = nil | |
| claim.LatestOccurrenceID = occurrenceID | |
| claim.UpdatedAt = now | |
| m.pluginKanbanClaims[key] = claim | |
| key := pluginKanbanClaimKey(occurrence.AutomationID, occurrence.DocumentID) | |
| if claim, exists := m.pluginKanbanClaims[key]; exists { | |
| claim.RunID = run.ID | |
| claim.WritebackAt = nil | |
| claim.LatestOccurrenceID = occurrenceID | |
| claim.UpdatedAt = now | |
| m.pluginKanbanClaims[key] = claim | |
| } | |
| return true, nil |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestrator/internal/store/plugin_kanban_occurrence.go` around lines 216 -
222, Guard the claim lookup in the in-memory update path before modifying or
writing it back: after computing the key with pluginKanbanClaimKey, verify the
claim exists in m.pluginKanbanClaims and return or otherwise skip the update
when absent. Only assign RunID, WritebackAt, LatestOccurrenceID, and UpdatedAt
and write back the claim when an existing claim is found, preserving its
identity fields.
| func (m *MemStore) MarkPluginKanbanOccurrenceReceipt(_ context.Context, occurrenceID, phase string, writtenAt *time.Time, writebackError string) error { | ||
| m.mu.Lock() | ||
| defer m.mu.Unlock() | ||
| occurrence, ok := m.pluginKanbanOccurrences[occurrenceID] | ||
| if !ok { | ||
| return ErrNotFound | ||
| } | ||
| if occurrence.ReceiptPhase != phase { | ||
| return ErrConflict | ||
| } | ||
| occurrence.WritebackError = writebackError | ||
| if writtenAt != nil { | ||
| at := *writtenAt | ||
| occurrence.ReceiptWrittenAt = &at | ||
| occurrence.WritebackError = "" | ||
| occurrence.UpdatedAt = at | ||
| } else { | ||
| occurrence.UpdatedAt = time.Now().UTC() | ||
| } | ||
| m.pluginKanbanOccurrences[occurrenceID] = occurrence | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Align MarkPluginKanbanOccurrenceReceipt semantics between the two stores.
The two implementations differ in three observable ways:
MemStoreforcesWritebackError = ""whenwrittenAt != nil.PGStorewrites thewritebackErrorargument verbatim.MemStoresetsUpdatedAt = *writtenAt.PGStoresetsupdated_at = now().MemStorereturnsErrNotFoundfor a missing occurrence andErrConflictfor a phase mismatch.PGStorereturnsErrConflictfor both, so a caller cannot distinguish a deleted occurrence from a phase race.
Memory-backed tests cannot detect these differences. Make PGStore clear writeback_error on a successful write and distinguish the missing-row case, or make MemStore follow the PostgreSQL behavior.
Also applies to: 780-792
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestrator/internal/store/plugin_kanban_occurrence.go` around lines 345 -
366, Update MemStore.MarkPluginKanbanOccurrenceReceipt to match PGStore
semantics: preserve the writebackError argument even when writtenAt is provided,
set UpdatedAt using the current UTC time rather than writtenAt, and return
ErrConflict for missing occurrences as well as phase mismatches.
| tag, err := tx.Exec(ctx, `UPDATE automation_kanban_claims | ||
| SET writeback_at=$4,updated_at=$4 | ||
| WHERE automation_id=$1 AND document_id=$2 | ||
| AND COALESCE(latest_occurrence_id,'')=$3 | ||
| AND writeback_at IS NULL`, | ||
| automationID, documentID, occurrenceID, at) | ||
| if err != nil { | ||
| return false, fmt.Errorf("mark Kanban Automation writeback: %w", err) | ||
| } | ||
| return tag.RowsAffected() == 1, nil | ||
| if tag.RowsAffected() == 0 { | ||
| return false, fmt.Errorf("mark Kanban Automation writeback: claim changed during occurrence update") | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Replayed Kanban writeback fails instead of returning a no-op. Both PostgreSQL writeback methods treat "no claim row matched" as a hard error, even when the occurrence block was skipped because occurrenceID is empty. A second call for an already-written claim, and a call for a missing claim, therefore return an error where MemStore returns (false, nil) or ErrNotFound. The reconciler drives both methods from a periodic tick, so every replay is logged as a failure and a genuine claim conflict is indistinguishable from it. The shared fix is to track whether the occurrence update was applied and to keep the hard error only for that case.
orchestrator/internal/store/plugins.go#L815-L826: add anoccurrenceUpdatedflag set after the occurrenceUPDATEsucceeds, and return(false, nil)when the claimUPDATEmatches 0 rows and the flag is false.orchestrator/internal/store/plugins.go#L867-L879: apply the identicaloccurrenceUpdatedguard toMarkPluginKanbanWritebackUnavailable.
📍 Affects 1 file
orchestrator/internal/store/plugins.go#L815-L826(this comment)orchestrator/internal/store/plugins.go#L867-L879
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@orchestrator/internal/store/plugins.go` around lines 815 - 826, Track whether
the occurrence UPDATE succeeds in both MarkPluginKanbanWriteback and
MarkPluginKanbanWritebackUnavailable at
orchestrator/internal/store/plugins.go:815-826 and 867-879. When the subsequent
claim UPDATE affects zero rows, return (false, nil) if the occurrence update was
skipped; retain the existing hard error when it was applied.
# Conflicts: # console/package.json # console/pnpm-lock.yaml
feat(runs): add provenance and Bot identity
feat(automations): add execution ledger
feat(usage): add auditable usage ledger
1 participant
Summary
Closes #14
Verification
Summary by CodeRabbit