Add project defaults to cycles #90

Merged

Breadway merged 3 commits from feature/bb-100-cycle-project-defaults into main 2026-10-15 13:01:22 +08:00

Conversation 3 Commits 3 Files changed 18 +811 -90

Breadway commented 2026-10-14 12:54:03 +08:00

Owner

Copy link

Closes #87

Cycles can select projects whose newly created issues inherit the cycle. Project selections can be edited later without changing historical issues. Explicit cycle assignments win; multiple matching open cycles return a conflict requiring explicit assignment. Includes migration, API/client/MCP surfaces, Settings UI, docs, and behavior tests.

Closes #87 Cycles can select projects whose newly created issues inherit the cycle. Project selections can be edited later without changing historical issues. Explicit cycle assignments win; multiple matching open cycles return a conflict requiring explicit assignment. Includes migration, API/client/MCP surfaces, Settings UI, docs, and behavior tests.

Breadway added 1 commit 2026-10-14 12:54:06 +08:00

Add project defaults to cycles 9c00c0e7be

Breadway added 1 commit 2026-10-14 13:00:11 +08:00

Fix cycle defaults regression coverage 49f72305ba

Breadway commented 2026-10-14 20:50:56 +08:00

Author

Owner

Copy link

Independent BB-100 review: CHANGES REQUESTED
Reviewed exact head 49f72305ba against base 5ff90e8cc2. All 14 changed files and complete 991-line diff reviewed in isolated ~/.worktrees/breadboard/fix/review-bb100-49f7230. Actual card #87 says any new issues under selected projects; interpretation includes newly created Forgejo-origin issues, not retroactive movement of historical imports.

Required repair checklist:

• R1 high, Settings.svelte:290-292: existing-cycle selector converts project_ids to strings while Svelte option values are numeric. Real built-SPA Chromium test: API [2] displays selectedOptions []; saving [2,3] reloads as []; choosing Beta from this visually empty selector persists [3], silently dropping Alpha. Keep option and selected value types consistent. Also fix rejected-save rollback: injected 400 leaves visible selection [2] while persisted selection remains [3]. Add browser tests for initial/reloaded display, additive editing, clear and rejected PATCH. Filed BB-106 (#93).
• R2 medium, store.rs:1107-1175 import_issue versus757-777 create_issue: newly opened Forgejo-origin issues bypass inheritance. Reproduced with enabled mapped Local/review project, sole open cycle selecting Alpha, signed issues/opened webhook with current timestamps: new external card cycle_id null, expected1. Cover webhook and reconcile for genuinely new external issues, define historical-import eligibility so old cards do not get reassigned, keep PR treatment explicit, and test conflict behavior. Filed BB-107 (#94). Repair on this existing PR.

Criterion mapping:
PASS API/database cycle selection at creation and replacement: migrations/0007_cycle_projects.sql; store.rs:435-557. Independent HTTP tests verified two projects, duplicate-id normalization, invalid create rollback, invalid patch rollback, missing-cycle404, unauthenticated PATCH401.
PASS normal create and Markdown import inheritance: store.rs:757-777, import.rs shared create_issue. Independent HTTP create by project key/id and checked/unchecked imports inherited the sole open cycle.
PASS explicit override and clear conflict: HTTP409 includes multiple-open-cycles diagnostic and rolls back number allocation; explicit cycle wins; closed cycles ignored. Open means closed=false, regardless of calendar dates.
PASS existing cards unchanged by selection edits: independent old unassigned, inherited, explicit and imported cards retained cycle_id after clearing defaults.
FAIL safe frontend editing: R1. Frontend creation did correctly persist selected numeric project ids.
FAIL any new issue across repository-origin creation: R2. Markdown import passes; Forgejo-origin new import fails.
PASS TS/MCP surfaces: api-client build, MCP build; actual stdio MCP tool discovery, create_cycle project_ids, update_cycle_projects by name and id, clear and end passed against local real API. Rust client update wrapper compiles in locked workspace tests/clippy, though no independent Rust HTTP wrapper test was added.

Independent checks: cargo test -p breadboard-server --locked --test api (4/4); cargo test --workspace --locked; cargo clippy --workspace --all-targets --locked -- -D warnings; cargo run -p xtask --locked -- check-docs; bash scripts/test-publish-bakery-stage.sh all pass locally. Clean npm ci, npm run build -w @breadboard/api-client, npm run check -w breadboard-web (0 errors/warnings), npm run build all pass. Initial web check before building api-client failed due to missing generated declarations, corrected by documented CI order, not a feature defect. Headless Chromium used only local scratch server, no live desktop/deployment. Evidence /tmp/opencode/bb100-independent2.log and worktree review-evidence/{ui.json,settings.png,server.log}; scripts /tmp/opencode/bb100-{independent.py,ui.mjs,mcp.mjs}.

Separate CI blocker, not a waiver: exact-head check / check (push) is FAILURE, run70. Local passing tests do not replace this. Coordinator reports shared shutdown.rs startup failure with PR88/89, prerequisite PR91 under repair. I verified failing exact-head status, but per-job/log API endpoints returned404 so detailed runner cause is not independently established here. Required check.yml includes clippy, test, docs, publisher, web. No branch protection entries; existing formal reviews empty. Integrate accepted prerequisite/current main, rerun all CI and request fresh review at resulting head. No merge or done.

npm audit reproduced eight advisories (1low/3moderate/3high/1critical), unchanged dependency lockfile. Deduplicated board and filed separate BB-108 (#95) for reachability/advisory triage, not a request for unrelated dependency churn here. Exploitability unverified.

Next: repair R1/R2 on PR90; BB-100 returns to todo; preserve branch and PR. Fresh exact-head review plus all green checks required before authorized merge. Coordinator session ses_ecaf08f38ffetTxrDPV5Z8cbkV. No implementation changes made by review. Markdown-import Forgejo writeback gap remains existing BB-96, not duplicated. History and current card body preserved. Models/session attribution not added. No merge authorization exercised. Progress is not completion. Preserved scratch evidence for rework. Version/release changes not performed. Primary checkout unchanged. Outstanding review state empty at read time, re-read again before any eventual merge. No approval is granted by this comment.

Independent BB-100 review: CHANGES REQUESTED Reviewed exact head 49f72305ba04e0694e472a03fc80949252921a59 against base 5ff90e8cc21b09fff09fd27b147e6f800d042a0e. All 14 changed files and complete 991-line diff reviewed in isolated ~/.worktrees/breadboard/fix/review-bb100-49f7230. Actual card #87 says any new issues under selected projects; interpretation includes newly created Forgejo-origin issues, not retroactive movement of historical imports. Required repair checklist: - [ ] R1 high, Settings.svelte:290-292: existing-cycle selector converts project_ids to strings while Svelte option values are numeric. Real built-SPA Chromium test: API [2] displays selectedOptions []; saving [2,3] reloads as []; choosing Beta from this visually empty selector persists [3], silently dropping Alpha. Keep option and selected value types consistent. Also fix rejected-save rollback: injected 400 leaves visible selection [2] while persisted selection remains [3]. Add browser tests for initial/reloaded display, additive editing, clear and rejected PATCH. Filed BB-106 (#93). - [ ] R2 medium, store.rs:1107-1175 import_issue versus757-777 create_issue: newly opened Forgejo-origin issues bypass inheritance. Reproduced with enabled mapped Local/review project, sole open cycle selecting Alpha, signed issues/opened webhook with current timestamps: new external card cycle_id null, expected1. Cover webhook and reconcile for genuinely new external issues, define historical-import eligibility so old cards do not get reassigned, keep PR treatment explicit, and test conflict behavior. Filed BB-107 (#94). Repair on this existing PR. Criterion mapping: PASS API/database cycle selection at creation and replacement: migrations/0007_cycle_projects.sql; store.rs:435-557. Independent HTTP tests verified two projects, duplicate-id normalization, invalid create rollback, invalid patch rollback, missing-cycle404, unauthenticated PATCH401. PASS normal create and Markdown import inheritance: store.rs:757-777, import.rs shared create_issue. Independent HTTP create by project key/id and checked/unchecked imports inherited the sole open cycle. PASS explicit override and clear conflict: HTTP409 includes multiple-open-cycles diagnostic and rolls back number allocation; explicit cycle wins; closed cycles ignored. Open means closed=false, regardless of calendar dates. PASS existing cards unchanged by selection edits: independent old unassigned, inherited, explicit and imported cards retained cycle_id after clearing defaults. FAIL safe frontend editing: R1. Frontend creation did correctly persist selected numeric project ids. FAIL any new issue across repository-origin creation: R2. Markdown import passes; Forgejo-origin new import fails. PASS TS/MCP surfaces: api-client build, MCP build; actual stdio MCP tool discovery, create_cycle project_ids, update_cycle_projects by name and id, clear and end passed against local real API. Rust client update wrapper compiles in locked workspace tests/clippy, though no independent Rust HTTP wrapper test was added. Independent checks: cargo test -p breadboard-server --locked --test api (4/4); cargo test --workspace --locked; cargo clippy --workspace --all-targets --locked -- -D warnings; cargo run -p xtask --locked -- check-docs; bash scripts/test-publish-bakery-stage.sh all pass locally. Clean npm ci, npm run build -w @breadboard/api-client, npm run check -w breadboard-web (0 errors/warnings), npm run build all pass. Initial web check before building api-client failed due to missing generated declarations, corrected by documented CI order, not a feature defect. Headless Chromium used only local scratch server, no live desktop/deployment. Evidence /tmp/opencode/bb100-independent2.log and worktree review-evidence/{ui.json,settings.png,server.log}; scripts /tmp/opencode/bb100-{independent.py,ui.mjs,mcp.mjs}. Separate CI blocker, not a waiver: exact-head check / check (push) is FAILURE, run70. Local passing tests do not replace this. Coordinator reports shared shutdown.rs startup failure with PR88/89, prerequisite PR91 under repair. I verified failing exact-head status, but per-job/log API endpoints returned404 so detailed runner cause is not independently established here. Required check.yml includes clippy, test, docs, publisher, web. No branch protection entries; existing formal reviews empty. Integrate accepted prerequisite/current main, rerun all CI and request fresh review at resulting head. No merge or done. npm audit reproduced eight advisories (1low/3moderate/3high/1critical), unchanged dependency lockfile. Deduplicated board and filed separate BB-108 (#95) for reachability/advisory triage, not a request for unrelated dependency churn here. Exploitability unverified. Next: repair R1/R2 on PR90; BB-100 returns to todo; preserve branch and PR. Fresh exact-head review plus all green checks required before authorized merge. Coordinator session ses_ecaf08f38ffetTxrDPV5Z8cbkV. No implementation changes made by review. Markdown-import Forgejo writeback gap remains existing BB-96, not duplicated. History and current card body preserved. Models/session attribution not added. No merge authorization exercised. Progress is not completion. Preserved scratch evidence for rework. Version/release changes not performed. Primary checkout unchanged. Outstanding review state empty at read time, re-read again before any eventual merge. No approval is granted by this comment.

Breadway added 1 commit 2026-10-15 12:19:38 +08:00

Repair cycle project defaults: numeric selector with rollback, external import inheritance ecdc070e5b

Breadway commented 2026-10-15 12:58:40 +08:00

Author

Owner

Copy link

Independent BB-100 re-review: PASS | Reviewed exact head ecdc070e5b against base 4fe391a90a (PR91 landed after the repair push; re-evaluated: zero file overlap with PR90, clean merge-tree, no re-review needed on that delta). All 18 changed files and the full 811+/90- diff reviewed in isolated ~/.worktrees/breadboard/fix/muse-rereview-pr90-ecdc070. Primary checkout untouched.

Acceptance (card 87: new issues under selected projects join the cycle; no retroactive moves):

• Local create: explicit cycle_id wins; otherwise the sole open cycle selecting the project is inherited; ambiguous defaults return 409; pre-existing issues untouched; PATCH replaces selections only, never moves issues. Verified by cycle_project_defaults_only_apply_to_new_issues_and_explicit_cycle_wins in crates/breadboard-server/tests/api.rs (passes in full suite).
• BB-106 (card 93): existing-cycle selector now uses bind:value with numeric ids and updateCycleProjects reloads Settings on success and on rejection. Browser suite apps/web/tests/cycle-defaults.browser.mjs (5 scenarios: initial render, additive edit, reload persistence, forced-400 rollback, clear) passes against a fresh build: cycle defaults browser regressions: passed. Same script against a 49f7230 build fails at step 1 with actual [] vs expected [ALPHA], confirming the old defect and the fix.
• BB-107 (card 94): import_issue inherits only for non-PR imports with both created_at and updated_at within 30 days, only into the unique open cycle default; historical, PR, ambiguous, and malformed-timestamp imports stay unassigned; update path never clobbers cycle_id (sync.rs has no cycle writes). New tests new_webhook_issues_inherit_unique_cycle_default_but_history_and_prs_do_not and reconcile_new_external_issues_inherit_project_cycle_default pass on this head and fail on 49f7230 (None vs Some(1)), confirming old defect and fix. Markdown imports route through create_issue, matching the documented rule.
• Migration 0007_cycle_projects.sql: composite PK, FK cascades, project index. Docs updated, check-docs passes.

Independent verification at ecdc070 (scratch worktree only):

• cargo test --locked --workspace: all suites ok (sync 12/12 incl. new tests; shutdown sigterm_exits_promptly_and_closes_sse_streams ran and passed locally).
• cargo clippy --workspace --all-targets --locked -- -D warnings: clean.
• check-docs: OK. scripts/test-publish-bakery-stage.sh: passed. npm ci, svelte-check (0 errors), vite build: clean.
• Exact-head CI: run 75 (head_sha ecdc070) status success, context check / check (push), 2m5s. Per-step logs are not exposed by the available API (jobs endpoints 404; web UI needs session auth), so each check step was re-run locally above instead of taking the combined status on faith. The single job runs clippy, cargo test (includes the shutdown test, not ignored), check-docs, bakery script, and web build.

Merge interplay: base moved 5ff90e8 to 4fe391a via PR91 (shutdown-test hardening only); no shared files, merge-tree clean. PR88 and PR89 heads are both CI-red (blocked on their own repair, not on this PR); merge-tree shows no textual conflicts between PR90 and either branch, so PR90 landing first does not block them. Recommendation: land PR90 first; PR88/PR89 need CI repair plus independent review before landing, in either relative order.

BB-108 (npm audit advisories, card 95) is pre-existing, untouched by this PR, and kept separate. No em dashes or attribution in commits or diff.

Independent BB-100 re-review: PASS | Reviewed exact head ecdc070e5b1f88b51893b19849b920eaa920d035 against base 4fe391a90a9f90d42d7eb9753c3e627aa004f0e9 (PR91 landed after the repair push; re-evaluated: zero file overlap with PR90, clean merge-tree, no re-review needed on that delta). All 18 changed files and the full 811+/90- diff reviewed in isolated ~/.worktrees/breadboard/fix/muse-rereview-pr90-ecdc070. Primary checkout untouched. Acceptance (card 87: new issues under selected projects join the cycle; no retroactive moves): - Local create: explicit cycle_id wins; otherwise the sole open cycle selecting the project is inherited; ambiguous defaults return 409; pre-existing issues untouched; PATCH replaces selections only, never moves issues. Verified by cycle_project_defaults_only_apply_to_new_issues_and_explicit_cycle_wins in crates/breadboard-server/tests/api.rs (passes in full suite). - BB-106 (card 93): existing-cycle selector now uses bind:value with numeric ids and updateCycleProjects reloads Settings on success and on rejection. Browser suite apps/web/tests/cycle-defaults.browser.mjs (5 scenarios: initial render, additive edit, reload persistence, forced-400 rollback, clear) passes against a fresh build: cycle defaults browser regressions: passed. Same script against a 49f7230 build fails at step 1 with actual [] vs expected [ALPHA], confirming the old defect and the fix. - BB-107 (card 94): import_issue inherits only for non-PR imports with both created_at and updated_at within 30 days, only into the unique open cycle default; historical, PR, ambiguous, and malformed-timestamp imports stay unassigned; update path never clobbers cycle_id (sync.rs has no cycle writes). New tests new_webhook_issues_inherit_unique_cycle_default_but_history_and_prs_do_not and reconcile_new_external_issues_inherit_project_cycle_default pass on this head and fail on 49f7230 (None vs Some(1)), confirming old defect and fix. Markdown imports route through create_issue, matching the documented rule. - Migration 0007_cycle_projects.sql: composite PK, FK cascades, project index. Docs updated, check-docs passes. Independent verification at ecdc070 (scratch worktree only): - cargo test --locked --workspace: all suites ok (sync 12/12 incl. new tests; shutdown sigterm_exits_promptly_and_closes_sse_streams ran and passed locally). - cargo clippy --workspace --all-targets --locked -- -D warnings: clean. - check-docs: OK. scripts/test-publish-bakery-stage.sh: passed. npm ci, svelte-check (0 errors), vite build: clean. - Exact-head CI: run 75 (head_sha ecdc070) status success, context check / check (push), 2m5s. Per-step logs are not exposed by the available API (jobs endpoints 404; web UI needs session auth), so each check step was re-run locally above instead of taking the combined status on faith. The single job runs clippy, cargo test (includes the shutdown test, not ignored), check-docs, bakery script, and web build. Merge interplay: base moved 5ff90e8 to 4fe391a via PR91 (shutdown-test hardening only); no shared files, merge-tree clean. PR88 and PR89 heads are both CI-red (blocked on their own repair, not on this PR); merge-tree shows no textual conflicts between PR90 and either branch, so PR90 landing first does not block them. Recommendation: land PR90 first; PR88/PR89 need CI repair plus independent review before landing, in either relative order. BB-108 (npm audit advisories, card 95) is pre-existing, untouched by this PR, and kept separate. No em dashes or attribution in commits or diff.

Breadway merged commit f02d977b1b into main 2026-10-15 13:01:22 +08:00

Breadway referenced this pull request from a commit 2026-10-15 13:01:23 +08:00

Merge pull request 'Add project defaults to cycles' (#90) from feature/bb-100-cycle-project-defaults into main

Breadway commented 2026-10-15 13:02:32 +08:00

Author

Owner

Copy link

Integration result: merged true. Merge commit f02d977b1b on main; reviewed head ecdc070e5b is an ancestor of the merge and head did not move between final review read and merge. Review evidence: issuecomment-5083. Residual race note: this Forgejo API version offers no expected-head merge condition; mitigated by the immediate pre-merge re-read (head/base/checks/reviews/mergeability) plus this post-merge confirmation. Post-merge check on main will run automatically (check.yml covers main pushes). Cards stay in review for coordinator verification; not marking done.

Integration result: merged true. Merge commit f02d977b1b3e3502b1c97bac5071944330b38edb on main; reviewed head ecdc070e5b1f88b51893b19849b920eaa920d035 is an ancestor of the merge and head did not move between final review read and merge. Review evidence: issuecomment-5083. Residual race note: this Forgejo API version offers no expected-head merge condition; mitigated by the immediate pre-merge re-read (head/base/checks/reviews/mergeability) plus this post-merge confirmation. Post-merge check on main will run automatically (check.yml covers main pushes). Cards stay in review for coordinator verification; not marking done.

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

  

ci

  

deploy

  

feature

  

opencode

  

polish

  

security

  

sync

  

test

  

ux

  

web

No labels

bug

ci

deploy

feature

opencode

polish

security

sync

test

ux

web

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Breadway/breadboard!90

No description provided.