feat: add phase 4 GitHub Projects provider#2558
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 |
|
Closed: this PR was opened against the upstream repository by mistake. The intended target is 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 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 42069b9. Configure here.
| if (!field) continue; | ||
| const name = trim(field.name) ?? trim(objectValue(field.field)?.name); | ||
| if (name && wanted.has(name.toLowerCase())) { | ||
| return field.value ?? field.name ?? field.text ?? field.title; |
There was a problem hiding this comment.
fieldValue fallback returns field name instead of value
Medium Severity
The fallback chain field.value ?? field.name ?? field.text ?? field.title uses field.name as a fallback, but field.name is the field's label (e.g. "Status", "Priority", "Agent") that was just matched on line 192. When field.value is undefined or null (e.g. a GitHub Project item with no status set), this returns the field label as the value. This causes incorrect mapping: toColumn("Status") yields "backlog", toAgent("Agent") yields "Human", and assigneeName returns "Assignee" instead of "Unassigned".
Reviewed by Cursor Bugbot for commit 42069b9. 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 direct input interpolation in workflow
Medium Severity
The expression '${{ inputs.findings_b64 }}' is directly interpolated into a shell run block. If the workflow_dispatch input contains a single quote, it breaks out of the quoted context and enables arbitrary command execution. The input should be passed via an environment variable instead of inline interpolation.
Triggered by project rule: Bugbot Project Brief
Reviewed by Cursor Bugbot for commit 42069b9. 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.
🟠 High src/kanbanConsoleMock.ts:586
When moving a task to "blocked" with confirmed: true, previewTaskTransition still returns requiresConfirmation: true and action: "open-action-sheet". This creates an infinite confirmation loop where users can never complete a blocked transition even after confirming. The condition at line 586 should check && !request.confirmed like the "done" case at line 574.
- if (request.toColumn === "blocked") {
+ if (request.toColumn === "blocked" && !request.confirmed) {🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/kanbanConsoleMock.ts around line 586:
When moving a task to `"blocked"` with `confirmed: true`, `previewTaskTransition` still returns `requiresConfirmation: true` and `action: "open-action-sheet"`. This creates an infinite confirmation loop where users can never complete a blocked transition even after confirming. The condition at line 586 should check `&& !request.confirmed` like the `"done"` case at line 574.
Evidence trail:
apps/web/src/kanbanConsoleMock.ts lines 574 (done case checks `!request.confirmed`), 586-595 (blocked case does NOT check `request.confirmed`), 603 (default case uses `!request.confirmed`). All at REVIEWED_COMMIT.
| 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).
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
gh authreadiness, Project/field/item reads, GitHub item to Kanban task mapping, confirmation-gated status updates, and confirmation-gated issue comments.updatedcontract to ISO datetime and align mock fixtures with the shared schema.Linked Work
Testing Guide
bun checkbun run --cwd apps/server test -- GitHubProjectsProviderbun run --cwd apps/server typecheckbun run --cwd packages/contracts test -- kanbanConsolebun run --cwd apps/web test -- kanbanConsoleMockgh auth statusandgh project list --owner MohAnghabo --limit 20 --format jsonRisks and Rollback
Readiness Checklist
Note
Medium Risk
Introduces a new GitHub Projects read/write surface (via
gh) plus new GitHub Actions workflows; incorrect CLI JSON assumptions or workflow misconfiguration could cause flaky behavior or unintended automation, though writes are confirmation-gated and the AI loop is disabled by default.Overview
Adds a server-side
GitHubProjectsProviderthat uses the GitHub CLI to check auth readiness, list Projects/fields/items, map Project items intoKanbanConsoleTasks, and (when explicitlyconfirmed) update Project status and post issue comments.Extends governance/agent infrastructure: introduces
.ai/rules/*guidance (PDPL/IFRS/security/environments/orchestration/PR-readiness), new Claude/Codex slash-command runbooks and wrappers, a PR readiness workflow, and optional AI review/auto-fix workflows controlled by.github/ai-loop.yml.Tightens/aligns supporting tooling: updates
AGENTS.mdand PR template for readiness gates, adds.local/to.gitignore, adjusts formatter ignore list, switches CI runners toubuntu-24.04, and adds a browser test for Arabic/RTL toggling inKanbanConsoleMock.Reviewed by Cursor Bugbot for commit 42069b9. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add GitHub Projects provider and AI loop infrastructure for the Kanban Console
GitHubProjectsProvider, an Effect-based service that wraps the GitHub CLI to list projects/fields/items, update item status, and post comments.KanbanConsoleMockReact component with drag-and-drop task movement, locale-aware RTL support, and a new/kanbanroute.ai-fix-router.yml,ai-fix-executor-claude.yml,ai-review.yml) that routes CI/review events to a Claude-based autofix executor; currently disabled via.github/ai-loop.yml.scripts/preflight/) with checks for Doppler, stack selection, environment tiers, and CLI versions, plus auto-fix helpers that can write secrets to Doppler.sync-codex-commands.ts.ai-loop.ymlgate must remainenabled: falseuntil intentionally activated.📊 Macroscope summarized 42069b9. 37 files reviewed, 11 issues evaluated, 4 issues filtered, 2 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 ]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. [ Failed validation ]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. [ Failed validation ]