feat(kanban): add phase 7 product artifacts#2566
Conversation
chore: adopt kanban governance baseline
…console feat(web): add phase 2 mock kanban console
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
Opened against upstream by mistake from the product fork workflow. Closing and recreating in MohAnghabo/kanban-console. |
github-actions
Bot
added
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cba9403. Configure here.
| shell: bash | ||
| run: | | ||
| findings_file="$RUNNER_TEMP/ai-loop-findings.json" | ||
| printf '%s' '${{ inputs.findings_b64 }}' | base64 --decode > "$findings_file" |
There was a problem hiding this comment.
Shell injection via unsanitized workflow dispatch input
Medium Severity
The ${{ inputs.findings_b64 }} value is interpolated directly into a shell command inside single quotes. If the input contains a single quote character, it breaks the quoting and enables arbitrary command injection. The safe pattern is to pass it through an environment variable (e.g., env: FINDINGS_B64: ${{ inputs.findings_b64 }}) and reference "$FINDINGS_B64" in the script instead. This violates the project's blocking rule against unsafe GitHub Actions patterns using event input in shell commands.
Triggered by project rule: Bugbot Project Brief
Reviewed by Cursor Bugbot for commit cba9403. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }; | ||
| } | ||
|
|
||
| if (request.toColumn === "blocked") { |
There was a problem hiding this comment.
🟢 Low src/kanbanConsoleMock.ts:683
The transition to "blocked" always returns requiresConfirmation: true regardless of whether request.confirmed is already true. Unlike the "done" case which checks && !request.confirmed, the "blocked" case has no such guard. This means users who have already confirmed will still be prompted again, creating an infinite confirmation loop.
- if (request.toColumn === "blocked") {
+ if (request.toColumn === "blocked" && !request.confirmed) {Also found in 1 other location(s)
scripts/preflight/checks/integrations.ts:181
The
doppler/yamlcheck provides a misleading hint even when the check passes. Whenfiles.length > 0 && !hasPlaceholderevaluates to true (status ="pass"), the hint is still set to"Replace placeholder Doppler project names."because the ternary at lines 182-185 only distinguishes betweenfiles.length === 0andfiles.length > 0, not whether the check passed. Other checks in this file correctly useundefinedfor the hint when passing (e.g., line 221, line 250), but this one always provides a string hint.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/kanbanConsoleMock.ts around line 683:
The transition to `"blocked"` always returns `requiresConfirmation: true` regardless of whether `request.confirmed` is already `true`. Unlike the `"done"` case which checks `&& !request.confirmed`, the `"blocked"` case has no such guard. This means users who have already confirmed will still be prompted again, creating an infinite confirmation loop.
Evidence trail:
apps/web/src/kanbanConsoleMock.ts lines 657-703 at REVIEWED_COMMIT: previewTaskTransition function. Line 672: `if (request.toColumn === "done" && !request.confirmed)` guards with `!request.confirmed`. Line 683: `if (request.toColumn === "blocked")` has no such guard. Line 700: default case uses `requiresConfirmation: !request.confirmed`. The asymmetry shows the "blocked" case is missing the `!request.confirmed` check.
Also found in 1 other location(s):
- scripts/preflight/checks/integrations.ts:181 -- The `doppler/yaml` check provides a misleading hint even when the check passes. When `files.length > 0 && !hasPlaceholder` evaluates to true (status = `"pass"`), the hint is still set to `"Replace placeholder Doppler project names."` because the ternary at lines 182-185 only distinguishes between `files.length === 0` and `files.length > 0`, not whether the check passed. Other checks in this file correctly use `undefined` for the hint when passing (e.g., line 221, line 250), but this one always provides a string hint.
| if ( | ||
| checks.some((check) => check.id === "stack-b/convex-deployment" && check.status === "error") | ||
| ) { | ||
| fixes.push(await runProviderCli(context, deps, "convex-dev")); | ||
| } | ||
|
|
||
| if (hasFixableIssue(checks, "stack-b/vercel-link")) { | ||
| fixes.push(await runProviderCli(context, deps, "vercel-link")); | ||
| } | ||
|
|
||
| if (checks.some((check) => check.id === "stack-a/neon-url" && check.status === "error")) { | ||
| fixes.push(await runProviderCli(context, deps, "neon-project-create")); | ||
| } |
There was a problem hiding this comment.
🟢 Low fix/apply.ts:34
The conditions for stack-b/convex-deployment (line 35) and stack-a/neon-url (line 44) check only status === "error" without verifying fixable, unlike other fixes that use hasFixableIssue. This causes runProviderCli to execute even when a check explicitly reports fixable: false, running potentially expensive or destructive CLI commands for issues marked as non-auto-fixable.
- if (
- checks.some((check) => check.id === "stack-b/convex-deployment" && check.status === "error")
- ) {
+ if (hasFixableIssue(checks, "stack-b/convex-deployment")) {
fixes.push(await runProviderCli(context, deps, "convex-dev"));
}
if (hasFixableIssue(checks, "stack-b/vercel-link")) {
fixes.push(await runProviderCli(context, deps, "vercel-link"));
}
- if (checks.some((check) => check.id === "stack-a/neon-url" && check.status === "error")) {
+ if (hasFixableIssue(checks, "stack-a/neon-url")) {
fixes.push(await runProviderCli(context, deps, "neon-project-create"));
}🤖 Copy this AI Prompt to have your agent fix this:
In file scripts/preflight/fix/apply.ts around lines 34-46:
The conditions for `stack-b/convex-deployment` (line 35) and `stack-a/neon-url` (line 44) check only `status === "error"` without verifying `fixable`, unlike other fixes that use `hasFixableIssue`. This causes `runProviderCli` to execute even when a check explicitly reports `fixable: false`, running potentially expensive or destructive CLI commands for issues marked as non-auto-fixable.
Evidence trail:
scripts/preflight/fix/apply.ts lines 10-14 (hasFixableIssue checks fixable), lines 34-38 (convex-deployment bypasses fixable), lines 44-45 (neon-url bypasses fixable), lines 26,30,40 (other fixes use hasFixableIssue). scripts/preflight/checks/support.ts line 37 (fixable defaults to false). scripts/preflight/checks/stack.ts lines 99-107 (convex-deployment check never sets fixable), lines 21-23 (neon-url error check never sets fixable), line 140 (vercel-link explicitly sets fixable: !linked).
| input.targetTag | ||
| ? readOptionalGitOutput(git, { | ||
| cwd: input.cwd, | ||
| operation: "KanbanGitStatusProvider.targetTag", | ||
| args: ["tag", "--list", input.targetTag], | ||
| }) | ||
| : Effect.succeed(""), |
There was a problem hiding this comment.
🟡 Medium kanban/GitStatusProvider.ts:535
When input.targetTag is undefined and targetTag is derived from the branch name, targetTagOutput is set to an empty string (via Effect.succeed("") on line 541) instead of checking if the derived tag exists. This causes the gate-tag-readiness gate to incorrectly report "passing" even when the derived tag already exists in the repository.
- input.targetTag
- ? readOptionalGitOutput(git, {
- cwd: input.cwd,
- operation: "KanbanGitStatusProvider.targetTag",
- args: ["tag", "--list", input.targetTag],
- })
- : Effect.succeed(""),🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/kanban/GitStatusProvider.ts around lines 535-541:
When `input.targetTag` is undefined and `targetTag` is derived from the branch name, `targetTagOutput` is set to an empty string (via `Effect.succeed("")` on line 541) instead of checking if the derived tag exists. This causes the `gate-tag-readiness` gate to incorrectly report "passing" even when the derived tag already exists in the repository.
Evidence trail:
apps/server/src/kanban/GitStatusProvider.ts lines 535-541 (targetTagOutput set to empty string when input.targetTag is falsy), line 554 (targetTag derived from branch via releaseTagFromBranch), lines 494-498 (releaseTagFromBranch returns a tag string for release/ branches), lines 582-586 (gate logic uses derived targetTag with unchecked targetTagOutput)
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Summary
docs/product/**/*.md.Linked Work
docs/tasks/t3-kanban-project-console.mdTesting Guide
bun run --cwd packages/contracts test -- kanbanConsolebun run --cwd apps/server test -- ProductArtifactsProviderbun run --cwd apps/web test -- kanbanConsoleMockbun run --cwd apps/web test:browser -- KanbanConsoleMockbun run fmt:checkgit diff --checkbun checkDocs and Tests Impact
docs/tasks/t3-kanban-project-console.md.Risks and Rollback
Well-Architected / Compliance
PR Size Rationale
Readiness Checklist
bun checkNote
Medium Risk
Adds new GitHub Actions workflows (PR readiness + optional AI review/auto-fix router/executor) and new server-side GitHub CLI integrations, which can affect CI behavior and repo automation if misconfigured; most automation is gated/disabled by default and mutating operations require explicit confirmation.
Overview
Introduces a governance/agent runbook kit under
.ai/and.claude/(PDPL + IFRS + security + PR-readiness + environment-topology rules) plus synced.codex/wrappers and a Codexenvironment.tomlfor consistent agent execution.Adds CI/automation scaffolding: a new
pr-readinessworkflow that enforces PR body checkboxes + required check-run names viascripts/check-pr-readiness.sh, plus optional AI review and closed-loop auto-fix workflows (.github/ai-loop.yml,ai-review,ai-fix-router,ai-fix-executor-claude) that are disabled by default.Extends the server Kanban slice with
GitHubProjectsProvider(read Projects viagh, update status, post comments) andAgentWorkflowLauncher(generate agent command recipes, build a redacted task context package, queue sessions with duplicate suppression, and post lifecycle comments), with tests emphasizing redaction and explicit confirmation for any write/comment action.Reviewed by Cursor Bugbot for commit cba9403. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add phase 7 product artifacts for Kanban console including mock UI, contracts, preflight checks, and AI loop workflows
/kanban.github/ai-loop.yml📊 Macroscope summarized cba9403. 38 files reviewed, 13 issues evaluated, 5 issues filtered, 3 comments posted
🗂️ Filtered Issues
apps/server/src/kanban/GitHubProjectsProvider.ts — 0 comments posted, 1 evaluated, 1 filtered
fieldValue, the priority of the nullish coalescing on line 191 is reversed. When the GitHub CLI returns field values with structure{ "field": { "name": "Status" }, "name": "In Progress" }, the expressiontrim(field.name) ?? trim(objectValue(field.field)?.name)evaluatesfield.namefirst, which yields the value ("In Progress") rather than the field name ("Status"). Since "in progress" is not in thewantedset for field names like "Status", the field is skipped and the lookup falls through to the fallback. The expression should betrim(objectValue(field.field)?.name) ?? trim(field.name)to prioritize the nestedfield.field.name(the actual field name) overfield.name(which may hold the value). [ Failed validation ]apps/server/src/kanban/ProductArtifactsProvider.ts — 0 comments posted, 1 evaluated, 1 filtered
statusFromPorcelainfails to detectDD(both deleted) andAA(both added) merge conflicts. Git porcelain v1 uses these status codes for unmerged entries that don't contain 'U':DDmeans both deleted,AAmeans both added. The checkline.slice(0, 2).includes("U")will return false for these, causing the function to incorrectly return"dirty"instead of"conflict". This could allowwriteArtifactto proceed on a file that is actually in a merge conflict state. [ Failed validation ]scripts/ai-loop/config.ts — 0 comments posted, 1 evaluated, 1 filtered
.github/ai-loop.yml(a YAML file), but the code usesJSON.parse()to parse it. YAML is a superset of JSON, meaning valid JSON is valid YAML, but not vice versa. If the config file uses any YAML-specific syntax (like unquoted strings, multi-line strings with|, anchors, etc.),JSON.parse()will throw aSyntaxError. Either the file extension should be.json, or a YAML parser library should be used. [ Out of scope (triage) ]scripts/ai-loop/router-logic.ts — 0 comments posted, 1 evaluated, 1 filtered
state.last_result_fingerprintbut the calling code stores the fingerprint instate.last_signal_fingerprint. Inrouter.ts, after computingfindingSetFingerprint, it is assigned tolast_signal_fingerprint(e.g.,last_signal_fingerprint: findingSetFingerprint). However,shouldBlockRepeatedFindingSetcompares againstlast_result_fingerprint, which is a different field that is never populated with the finding fingerprint. This means the repeated-finding-set detection will never trigger, defeating the intended blocking behavior. [ Failed validation ]scripts/preflight/checks/integrations.ts — 0 comments posted, 1 evaluated, 1 filtered
doppler/yamlcheck provides a misleading hint even when the check passes. Whenfiles.length > 0 && !hasPlaceholderevaluates to true (status ="pass"), the hint is still set to"Replace placeholder Doppler project names."because the ternary at lines 182-185 only distinguishes betweenfiles.length === 0andfiles.length > 0, not whether the check passed. Other checks in this file correctly useundefinedfor the hint when passing (e.g., line 221, line 250), but this one always provides a string hint. [ Cross-file consolidated ]