chore(requirements): replace nightly sync with a command, reconcile DB with the board - #905
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds GitHub Project ingestion, transactional requirements reconciliation, and source-aware ticket analysis. Replaces the nightly synchronisation workflow with a Claude command and updates related documentation. ChangesRequirements tooling
Sequence Diagram(s)sequenceDiagram
participant Developer
participant qk_requirements_sync
participant GitHubProject43
participant requirements_db
participant GitHubPullRequest
Developer->>qk_requirements_sync: Run synchronisation command
qk_requirements_sync->>GitHubProject43: Fetch board items
GitHubProject43-->>qk_requirements_sync: JSONL requirements data
qk_requirements_sync->>requirements_db: Rebuild and verify migration
qk_requirements_sync->>GitHubPullRequest: Commit migration and open pull request
GitHubPullRequest-->>Developer: Return pull-request URL
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
🎭 Playwright E2E Test Results82 tests 52 ✅ 4m 1s ⏱️ Results for commit 43c52f1. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
requirements/scripts/generate_sync_migration.ts (1)
157-171: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winTitle and statement drift is never reconciled.
Only
status,priority,granularityand the two impl fields are diffed, so an issue retitled or rewritten on GitHub leaves the database permanently stale — and the parity check inqk-requirements-sync.mdStep 4 compares status only, so it will still report a clean mirror. Either difftitle/statementtoo, or state the exclusion explicitly in the file header alongside therequirement_linkcarve-out.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2fe0d4b8-1ae3-4b83-8dc3-4cc307ec1719
⛔ Files ignored due to path filters (1)
requirements/requirements.dbis excluded by!**/*.db
📒 Files selected for processing (7)
.claude/commands/multi-agent/qk-requirements-sync.md.claude/commands/multi-agent/qk-tickets.md.github/workflows/requirements-sync.ymldocs/GITHUB_MCP.mdrequirements/migrations/011_reconcile_board_2026_07_29.sqlrequirements/scripts/fetch_board.shrequirements/scripts/generate_sync_migration.ts
💤 Files with no reviewable changes (1)
- .github/workflows/requirements-sync.yml
| '2025-10-10T11:12:59Z', '2025-10-10T11:12:59Z', 'github-actions[bot]', 'github-actions[bot]'); | ||
|
|
||
| INSERT INTO requirement_change | ||
| (requirement_id, version, change_type, change_summary, changed_by, changed_at) | ||
| VALUES | ||
| (160, 1, 'created', 'imported from GitHub issue', 'github-actions[bot]', '2025-10-10T11:12:59Z'); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Provenance is recorded as github-actions[bot], but this migration is now produced by an interactive command.
The nightly Action is gone; created_by/updated_by/changed_by should reflect the actual origin (e.g. qk-requirements-sync) so the audit trail does not point at a workflow that no longer exists. Root cause is the CHANGED_BY constant in requirements/scripts/generate_sync_migration.ts (line 17) — changing it there regenerates this file correctly.
| // Generate a migration that reconciles the requirements database with the board. | ||
| // | ||
| // Reads board state (JSON Lines from fetch_board.sh) plus the built database, and | ||
| // emits SQL for the delta on stdout. Emits nothing and exits 1 when there is no | ||
| // drift, so a caller can distinguish "nothing to do" from "something to write". | ||
| // | ||
| // Everything here is mechanical: status comes from the board column, priority and | ||
| // granularity from labels, impl fields from merged closing PRs. Inferred | ||
| // requirement_link rows are deliberately NOT generated — those are judgement and | ||
| // are added as a separate reviewable step. | ||
| // | ||
| // Usage: tsx requirements/scripts/generate_sync_migration.ts <board.jsonl> <db> <today-iso> | ||
|
|
||
| import { execFileSync } from "node:child_process"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
File name should be kebab-case.
generate_sync_migration.ts uses snake_case; rename to generate-sync-migration.ts (and update the usage comment plus the npx tsx … invocation in .claude/commands/multi-agent/qk-requirements-sync.md Step 3). As per coding guidelines: "Use kebab-case for file and directory names."
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 13-13: Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFileSync } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
Source: Coding guidelines
| function queryDb(dbPath: string, sql: string): DbRow[] { | ||
| const out = execFileSync("sqlite3", ["-json", dbPath, sql], { encoding: "utf8" }).trim(); | ||
| return out === "" ? [] : JSON.parse(out); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js child_process execFileSync default maxBuffer size
💡 Result:
The default maxBuffer size for child_process.execFileSync in Node.js is 1024 * 1024 bytes (1 MiB) [1][2]. While older versions of Node.js previously used a default of 200 * 1024 bytes (200 KiB) following a change in 2019 [3][4], the current standard documented in recent versions of Node.js is 1024 * 1024 bytes [1][2]. The maxBuffer option specifies the largest amount of data in bytes allowed on stdout or stderr; if this limit is exceeded, the child process is terminated [1].
Citations:
- 1: https://nodejs.org/api/child_process.html
- 2: https://nodejs.org/docs/latest-v24.x/api/child_process.html
- 3: nodejs/node@ceb80f4
- 4: nodejs/node@eb8a51a35c
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline requirements/scripts/generate_sync_migration.ts --view expanded || true
echo "== relevant queryDb/query usage =="
sed -n '1,140p' requirements/scripts/generate_sync_migration.ts
echo "== migrate requirements migration 011 and impl_paths occurrences in repository =="
git ls-files | rg '011|migration|requirements' | head -n 120
rg -n "impl_paths|migration 011|migration/011|011" . --glob '!node_modules' --glob '!dist' --glob '!build' | head -n 200
echo "== read package/engine context =="
for f in package.json requirements/prisma/schema.prisma; do
[ -f "$f" ] && { echo "--- $f"; sed -n '1,220p' "$f"; }
doneRepository: hmcts/cath-service
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json, re, pathlib
migration = pathlib.Path("requirements/migrations/010_sync_github_2026_07_26.sql").read_text()
impl_paths = re.findall(r"impl_paths\s*=\.{0,1}\(\d, 1, 'impl_paths', NULL, '(\[.*?\])'", migration, re.S)
if not impl_paths:
impl_paths = re.findall(r"impl_paths\s*,\s*NULL,\s*'(\[.*?\])'", migration, re.S)
print("impl_paths occurrences:", len(impl_paths))
sizes = []
paths = []
for s in impl_paths:
sizes.append(len(s.encode("utf-8")))
paths.append(json.loads(s))
print("total bytes:", sum(sizes))
print("average bytes:", sum(sizes) / len(sizes))
print("max rows:", len(sizes))
print("max entry bytes:", max(sizes))
print("max entry paths:", max(paths, key=len))
print("node default maxBuffer byte:", 1024*1024)
print("rows_needed_to_exceed_1mb_5k_rows:", 1024*1024 // 5000 + 1)
print("rows_needed_to_exceed_1mb_2_6mb_rows:", 1024*1024 // 2600000 + 1)
PY
echo "== current queryDb =="
sed -n '80,88p' requirements/scripts/generate_sync_migration.tsRepository: hmcts/cath-service
Length of output: 307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib
migration = pathlib.Path("requirements/migrations/010_sync_github_2026_07_26.sql").read_text()
print("migration size:", len(migration.encode("utf-8")))
print("contains 367:", "367:" in migration)
print("contains REQ-0099:", "REQ-0099" in migration)
candidates = migration[migation.index("367"):migation.index("367") + 1000] if "367:" in migration else migration[:2000]
print("first 2000 chars with line numbers:")
for i, line in enumerate(migration.splitlines()[:25], 1):
if "impl_paths" in line or "REQ-0099" in line or "created_at, updated_at" in line or "367:" in line:
print(i, line[:280])
PY
# Minimal behaviour check for execFileSync maxBuffer option shape.
node - <<'JS'
const { execFileSync } = require('node:child_process');
const out = execFileSync("node", ["-e", "console.log('x');"], { encoding: "utf8", maxBuffer: 16 * 1024 * 1024 });
console.log("maxBuffer option accepted:", out.trim() === "x");
JS
echo "== current queryDb =="
sed -n '80,88p' requirements/scripts/generate_sync_migration.tsRepository: hmcts/cath-service
Length of output: 731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
migration=$(awk '1==1 && /UPDATE "requirement"/{flag=1} flag<=4{print} /WHERE id = 99/{flag2=1} flag2{print; flag2=0} END{print "END"}' requirements/migrations/010_sync_github_2026_07_26.sql | sed -n '320,390p')
echo "$migration"
python3 - <<'PY'
import pathlib, re, json
migration = pathlib.Path("requirements/migrations/010_sync_github_2026_07_26.sql").read_text()
paths = re.findall(r"'(\[.*?\])'", migration)
impl_paths = [p for p in paths if '"/' in p or ".'" in p]
print("string-array candidates:", len(paths))
print("impl_paths-like candidates:", len(impl_paths))
for p in impl_paths[:5]:
print("length:", len(p.encode(), "utf-8"))
data = json.loads(p)
print("paths:", data[:3] if data else [])
# Also parse line-by-line UPDATE format because the JSON regex depends on implementation detail.
for line in migration.splitlines():
if "UPDATE \"requirement\"" in line or "impl_paths" in line or "WHERE id = 99" in line:
print(line[:280])
PYRepository: hmcts/cath-service
Length of output: 5471
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all lines containing id 859 and impl_paths assignments with adjacent context =="
rg -n -C 3 "id = 859|859,'|impl_paths" requirements/migrations/010_sync_github_2026_07_26.sql requirements/migrations/011_reconcile_board_2026_07_29.sql requirements/schema.sql
echo "== migration line counts =="
wc -l requirements/migrations/010_sync_github_2026_07_26.sql requirements/migrations/011_reconcile_board_2026_07_29.sql requirements/schema.sqlRepository: hmcts/cath-service
Length of output: 50375
Set an explicit maxBuffer for the sqlite3 query.
The first queryDb() returns every requirement with its impl_paths JSON, and existing migrations already show multi-KB path arrays. As the requirement set grows, this output can exceed execFileSync’s default 1 MiB buffer and fail with ENOBUFS before drift is reported. Set a generous explicit maxBuffer.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7456424e-7ea9-4c3f-9cd7-17fe8ec6617e
📒 Files selected for processing (4)
.claude/commands/multi-agent/qk-requirements-sync.md.claude/commands/multi-agent/qk-tickets.mdrequirements/migrations/011_reconcile_board_2026_07_29.sqlrequirements/scripts/fetch_board.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- requirements/scripts/fetch_board.sh
- requirements/migrations/011_reconcile_board_2026_07_29.sql
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.claude/commands/multi-agent/qk-requirements-sync.md (2)
200-212: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winFail when SQLite verification reports a failure.
set -eonly checks thesqlite3process exit code.PRAGMA integrity_checkcan printnot ok, andPRAGMA foreign_key_checkcan print violations whilesqlite3still exits successfully. The command can therefore continue with an invalid database.Capture both outputs and exit unless the integrity result is exactly
okand the foreign-key result is empty.
214-232: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the parity check complete and enforce its result.
The query compares only
issue_numberandstatus, although Step 4 also reconcilespriority,granularity,impl_commit_sha, andimpl_paths. Incorrect values in those fields can pass verification. The block also prints non-empty arrays without failing.Compare every reconciled field and use
jq -eor an equivalent shell assertion before opening the PR.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ce771d0-b2c6-4516-a05e-1be6186b9eb5
📒 Files selected for processing (2)
.claude/commands/multi-agent/qk-requirements-sync.md.claude/commands/multi-agent/qk-tickets.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .claude/commands/multi-agent/qk-tickets.md
…ith board The nightly requirements-sync Action ran with a GitHub App token that cannot read Project #43: HMCTSClaudeCode declares repository_projects (Projects classic) but no organisation Projects permission, and ProjectV2 is an org-scoped resource. The ProjectV2 query returned FORBIDDEN, and rather than stopping, the job substituted "issue closed + merged closing PR" as a proxy for board status. That proxy cannot see open issues, so every ticket in Refined Tickets was invisible to it. Migrations 004-010 each shipped carrying "Board access unavailable / HUMAN REVIEW REQUIRED", the run exited green every night, and the database fell 126 requirements behind the board. Rather than widen an org-wide App's permissions to fix an unattended job, move the sync to an interactive command where a developer's own token already has read:project. - Delete .github/workflows/requirements-sync.yml. - Add requirements/scripts/fetch_board.sh: pages the board and maps each column to a status. Exits non-zero if the board is unreadable, including the data.node == null case a token without projects access actually returns instead of an HTTP error. An unreadable board now stops the run. - Add requirements/scripts/generate_sync_migration.ts: emits the delta as SQL, exits 1 when there is none. Output is deterministic, so a rerun on unchanged input regenerates byte-identical SQL. - Add /qk-requirements-sync, which fetches the board, generates the migration, infers requirement_link rows for new requirements, verifies the mirror is exact, and opens a PR. - Add migration 011: 127 new requirements (REQ-0160..REQ-0286), 25 updated. Every board issue now has a matching row with a matching status. The column mapping covers the whole board, not just Refined Tickets and beyond. Backlog maps to draft and Prioritised Backlog to proposed, matching what seed.sql already did for those columns, so a ticket moving backwards is mirrored like any other move and recorded in requirement_change. The old never-down-status rule existed to protect against guessed data; with a readable board it just held the database out of date. /qk-tickets: its query referenced story_points and assigned_to, which are not columns in schema.sql, so it errored out entirely; it also described 'blocks' and 'related_to' link types that are not in the CHECK constraint. Group by priority instead, use the real link types, and add a source argument so it can read either master or an open sync PR's migration. It now also works out which approved tickets are safe to run in parallel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Biome lint/style/useTemplate. Generated SQL is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The database is derived entirely from schema.sql + seed.sql + migrations/*.sql, which init_db.sh applies in order. The migrations describe what is in it, so the binary adds nothing a clone cannot rebuild — and committing it means every sync PR carries a ~2MB binary diff that no reviewer can read. .gitignore has listed requirements/*.db since #695, but the file was already tracked by then: it was swept into #772 (an unrelated list-types PR) the same day, and gitignore does not apply to tracked files. Remove it from the index; it stays on disk and is now correctly ignored. Rebuild with: yarn requirements:build Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fetch_board.sh - Fail on any board column with no status mapping, naming the column and the offending issues. Previously such items were filtered out silently, which is the same "quietly incomplete" failure the script exists to prevent: a renamed column would drop items from the feed, and the Step 4 parity check only compares issues that are in the feed, so it would still pass. - Define the column mapping once and share it between the guard and the emit step so the two cannot drift apart. - Use mktemp plus a trap for the stderr capture instead of a predictable /tmp/...$$ path that leaked on the early-exit branches. - Count only issue-bearing items, and report how many were mapped. qk-tickets - Build in a temporary worktree at origin/master. Previously `master` built from whichever branch was checked out, and the PR path overlaid onto that, so a feature branch could produce a database that was neither master nor master plus the PR. - This also removes the dangerous cleanup: `git checkout -- requirements/` would have discarded a user's own tracked edits while failing to delete an untracked overlaid migration. Cleanup is now removing the worktree, and it asserts the main checkout is untouched. - Normalise `pr <number>` to a bare number before use; `$ARGUMENTS` is not positional, so `$1` was never set. - Separate `gh pr diff`'s exit status from grep's no-match, so an auth or network failure is not reported as "this PR has no migrations". - Compute the transitive closure with a recursive CTE. The adjacent-links queries missed paths through non-approved requirements, so two tickets linked via an intermediate could be called safe to parallelise. - Point the agent at the worktree database, not the main checkout's. - Fix a reference to "Step 6"; the restore step is Step 4. qk-requirements-sync - Fetch the remote branch and branch from origin/ when reusing an open sync PR; the previous checkout assumed a local ref that may not exist. Migration 011 and the command now record provenance as 'qk-requirements-sync' rather than 'github-actions[bot]', which pointed at a workflow this PR deletes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two fixes from the second review pass. qk-tickets: the recursive CTE walked through requirements of any status, which contradicted the rule stated a few lines below it — that a dependency on a verified requirement is already satisfied and does not block. So approved A -> verified X -> approved B reported A and B as mutually blocking when the only thing between them was finished work. On the current link graph 92 of 187 transitive paths run through a verified node, so this materially over-constrained the parallel set. Traversal now stops AT a verified requirement instead of through it. Verified nodes still appear as directly-reached, which is what lets Step 3 report the dependency as satisfied. Verified against a synthetic graph: A -> verified X -> B does not link A to B, while A -> draft Y -> C still does. qk-requirements-sync: stage the one migration path this run produced rather than `git add requirements/migrations/`, which would sweep in any other edit or stray file in that directory, and refuse to commit if anything else is staged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An unmerged sync PR is not in master, so the next run rebuilt from master, re-detected everything that PR had already added as still missing, and handed out the same ids and REQ-NNNN refs a second time. id is the primary key and ref is UNIQUE, so both migrations could never apply together: init_db.sh fails and the database stops building. The old Step 6 branch-reuse looked like it covered this, but by then the SQL was already numbered against master. Move the check to Step 1, before the board fetch and the delta, and stop rather than try to extend the open PR — keeping ref allocation single-writer is the simplest way to guarantee a ref is never issued twice. Step 6 re-checks, since a PR can be opened while a run is in progress. Match the sync branch shape exactly (chore/requirements-sync-<YYYY-MM-DD>), not just its prefix: a hand-made branch like chore/requirements-sync-rework would otherwise block every sync. Same pattern applied to the qk-tickets PR list. Step 2b also now requires requirements/ to be clean, since a stray local migration would shift MAX(id) and the ref sequence. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ongoing deltas are a few rows at a time, so a 242-line generator to emit them was more machinery than the job needs. Delete generate_sync_migration.ts and update migration 011's header to say it was machine-generated rather than pointing at a deleted script. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
43c52f1 to
9992679
Compare
There was a problem hiding this comment.
Actionable comments posted: 8
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a574499-8e56-48bd-a887-e3247da41f29
⛔ Files ignored due to path filters (1)
requirements/requirements.dbis excluded by!**/*.db
📒 Files selected for processing (6)
.claude/commands/multi-agent/qk-requirements-sync.md.claude/commands/multi-agent/qk-tickets.md.github/workflows/requirements-sync.ymldocs/GITHUB_MCP.mdrequirements/migrations/011_reconcile_board_2026_07_29.sqlrequirements/scripts/fetch_board.sh
💤 Files with no reviewable changes (1)
- .github/workflows/requirements-sync.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/GITHUB_MCP.md
| # Re-check: the guard ran before the board fetch and delta, so a PR could have been | ||
| # opened in between. Cheap to repeat, and the alternative is colliding refs. | ||
| # Use the same date-shaped regex as Step 1 so rework/manual branches are not mistaken | ||
| # for a sync in flight. | ||
| RECHECK=$(gh pr list --state open --search "head:chore/requirements-sync-" \ | ||
| --json number,headRefName \ | ||
| --jq '.[] | select(.headRefName | test("^chore/requirements-sync-[0-9]{4}-[0-9]{2}-[0-9]{2}$")) | .number' \ | ||
| | head -1) | ||
| if [ -n "$RECHECK" ]; then | ||
| echo "A sync PR was opened while this run was in progress — discarding this migration." | ||
| echo "Remove ${MIGRATION}, then re-run once that PR is merged or closed." | ||
| exit 1 | ||
| fi | ||
|
|
||
| # The migration is untracked, so it survives the branch switch. | ||
| git fetch --quiet origin master | ||
| git checkout -b "$BRANCH" origin/master |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Serialise concurrent sync runs before allocating IDs.
The open-PR checks are not a lock. Two invocations can pass both checks before either PR is visible, build the same origin/master baseline, allocate the same IDs and REQ references, and race on the date branch. Add an atomic local or remote lock before generating the migration. Do not rely on the recheck.
| WITH RECURSIVE reachable(root_id, id, depth) AS ( | ||
| SELECT r.id, r.id, 0 | ||
| FROM requirement r | ||
| WHERE r.status = 'approved' AND r.issue_number IS NOT NULL | ||
| UNION | ||
| SELECT rc.root_id, rl.target_id, rc.depth + 1 | ||
| FROM reachable rc | ||
| JOIN requirement mid ON mid.id = rc.id | ||
| JOIN requirement_link rl ON rl.source_id = rc.id | ||
| WHERE rl.type IN ('depends_on', 'derives_from', 'refines') | ||
| AND rc.depth < 20 | ||
| AND (rc.depth = 0 OR mid.status <> 'verified') | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve dependency paths deeper than 20.
depth is part of the recursive row, so UNION does not deduplicate a node revisited at a new depth. The depth limit prevents non-termination, but it also hides valid dependency paths longer than 20. Those tickets can then be reported as safe to parallelise. Track visited node IDs in the recursive state. If traversal is truncated, mark the result incomplete and do not report the pair as safe.
Also applies to: 230-233
| INSERT INTO requirement | ||
| (id, ref, title, statement, kind, status, priority, granularity, | ||
| issue_number, issue_url, impl_commit_sha, impl_paths, | ||
| created_at, updated_at, created_by, updated_by) | ||
| VALUES | ||
| (221, 'REQ-0221', 'Implement Third-Party Inbound Publication API', '## User Story | ||
|
|
||
| As a platform engineer, I want cath-service to expose a standardised inbound REST API so that CaTH can push new, updat |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Rebuild the requirements database from SQL sources and check version literals in migration 011.
set -euo pipefail
fd -t f . requirements --extension sql --extension md | sort
python3 - <<'PY'
import pathlib, re, collections
mig = pathlib.Path("requirements/migrations/011_reconcile_board_2026_07_29.sql")
text = mig.read_text(encoding="utf-8", errors="replace")
# Target version and id per UPDATE block.
updates = re.findall(
r"UPDATE requirement SET.*?version\s*=\s*(\d+).*?WHERE id\s*=\s*(\d+);",
text, re.S)
print("UPDATE (id -> new version):")
for ver, rid in updates:
print(f" id={rid} -> version={ver}")
# Versions asserted in the paired change rows.
changes = re.findall(
r"INSERT INTO requirement_change\s*\([^)]*\)\s*VALUES\s*\(\s*(\d+)\s*,\s*(\d+)\s*,",
text)
pairs = collections.Counter(changes)
dupes = {k: v for k, v in pairs.items() if v > 1}
print("\nDuplicate (requirement_id, version) audit pairs inside this migration:",
dupes or "none")
# Cross-check that each UPDATE has a matching change row at the same version.
upd = {(rid, ver) for ver, rid in updates}
chg = set(changes)
print("\nUPDATE rows with no matching change row:", sorted(upd - chg) or "none")
PY
# Show the highest version each requirement reaches in migrations 001-010.
fd -e sql . requirements/migrations --exec rg -n --with-filename 'version\s*=\s*[0-9]+' {} \
| rg -v '011_reconcile_board' || trueRepository: hmcts/cath-service
Length of output: 2980
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import pathlib, re, sqlite3
schema = pathlib.Path("requirements/schema.sql").read_text()
seed = pathlib.Path("requirements/seed.sql").read_text()
conn = sqlite3.connect(":memory:")
conn.executescript(schema)
conn.executescript(seed)
for mig in sorted(pathlib.Path("requirements/migrations")
.glob("00*.sql"))[:11]:
text2 = mig.read_text(errors="replace").replace("CREATE OR REPLACE FUNCTION", "CREATE FUNCTION")
conn.executescript(text2)
rows = conn.execute("SELECT id, version FROM requirement ORDER BY id").fetchall()
print("requirement id -> version after migrations 001-011:")
for r in rows:
print(f" id={r[0]} version={r[1]}")
required = {2,7,78,99,100,101,102,103,104,105,106,107,108,109,110,112,124,126,129,130,131,135,157,158,159}
actual = {r[0] for r in rows}
print("\nIDs updated in 011 present after migration replay:", required <= actual)
mig11 = pathlib.Path("requirements/migrations/011_reconcile_board_2026_07_29.sql").read_text()
updates = re.findall(r"UPDATE requirement SET.*?id\s*=\s*(\d+);", mig11, re.S)
versions_on_011 = re.findall(r"version\s*=\s*(\d+).*?WHERE id\s*=\s*(\d+);", mig11, re.S)
for version, rid in versions_on_011:
current = next((r[1] for r in rows if r[0] == int(rid)), None)
print(f"id={rid}: 011 sets version={version}; replayed version={current}; match={version == str(current)}")
# Full replay of migrations 001-011.
conn2 = sqlite3.connect(":memory:")
conn2.executescript(schema)
conn2.executescript(seed)
for mig in sorted(pathlib.Path("requirements/migrations").glob("0*.*sql"))[:11]:
text = mig.read_text(errors="replace").replace("CREATE OR REPLACE FUNCTION", "CREATE FUNCTION")
conn2.executescript(text)
print("\nAll updates in 011 match final replayed versions:", all(
row[1] == target_new for row in conn2.execute("SELECT id, version FROM requirement WHERE id IN (" + ",".join(updates or ["0"]) +")")
for target_new, row in [(next(v for v,_ in versions_on_011 if _==u), row)]
for u in [updates[versions_on_011.index((target_new, row[0]))]]
))
PYRepository: hmcts/cath-service
Length of output: 4856
Base migration 011 on the current requirement versions.
The hardcoded target versions in requirements/migrations/011_reconcile_board_2026_07_29.sql do not match the versions produced by replaying migrations 001-011. Several target rows update non-existent requirements, and the update for id=7 can regress requirement.version from 2 to 2 before later migrations expect subsequent increments. Derive these literals from current audit/version state instead of a stale snapshot, and consider version = version + 1 for future reconciliations.
| labels(first: 50) { nodes { name } } | ||
| closedByPullRequestsReferences(first: 20, includeClosedPrs: true) { | ||
| nodes { | ||
| number | ||
| state | ||
| mergeCommit { oid } | ||
| files(first: 100) { nodes { path } } | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Paginate every nested Project connection.
labels(first: 50), closedByPullRequestsReferences(first: 20), and files(first: 100) fetch only their first page. The query does not inspect pageInfo. The synchronisation command can therefore derive incorrect priority, granularity, impl_commit_sha, or impl_paths while the fetch still succeeds. Add nested pagination or fail when hasNextPage is true.
129 new requirements (REQ-0160–REQ-0288), 17 status/field changes, 110 inferred dependency links. Parity check passed against board. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
011 is already taken by the July 29 catch-up. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… 011 Migration 012 was generated from master without 011 applied, so it assigned ids 160-288 which 011 had already claimed. Shifted all new requirement ids and refs by +129 (289-417, REQ-0289 to REQ-0417) and updated the corresponding requirement_change and requirement_link references. Build now passes: 415 requirements, integrity_check ok. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The unconfirmed-link display section references both fields but they were missing from the SELECT lists. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
Why
The nightly
requirements-sync.ymlAction ran with a GitHub App token that cannot read Project #43.HMCTSClaudeCodedeclaresrepository_projects(Projects classic) but no organisation Projects permission, and ProjectV2 is an org-scoped resource — so the board query returnedFORBIDDEN.Rather than stopping, the job substituted its own proxy for board status: issue closed + merged closing PR. That proxy can only ever detect
verified, and it cannot see open issues at all — i.e. every ticket in Refined Tickets. Migrations 004–010 each shipped carryingHUMAN REVIEW REQUIRED: Board access unavailable, every run exited green, and the database fell 127 requirements behind the board.Fixing the App would mean widening an org-wide App's permissions across all 36
hmctsboards to serve one unattended job. Moving the sync to an interactive command avoids that entirely — a developer's own token already hasread:project.What changed
1. Nightly Action deleted —
.github/workflows/requirements-sync.yml(−212). Stale reference indocs/GITHUB_MCP.mdupdated.2. Migration 011 reconciles the DB with the board — 127 new requirements (REQ-0160…REQ-0286), 25 updated. The DB is now an exact mirror: 0 status mismatches, 0 board issues missing.
3.
/qk-requirements-sync(.claude/commands/multi-agent/), backed by one script:requirements/scripts/fetch_board.shpages the board and maps columns to statuses. It hard-fails on GraphQL error, ondata.node == null(what a token without projects access actually returns instead of an HTTP error), and on zero items. An unreadable board now stops the run instead of degrading it — that's the property whose absence caused this whole problem.The migration SQL is written directly by the command rather than by a generator: deltas are normally a handful of rows (002–010 inserted 1–5 requirements each), so the command carries the conventions instead — label mappings,
impl_pathsexclusions, id/ref arithmetic, one-UPDATE-per-requirement versioning, quote escaping. It also forbids substituting any proxy for board status, and verifies the DB mirrors the board exactly before opening a PR — the check the old Action never had. Migration 011 was generated mechanically as a one-off, being 127 rows with ~1,500 apostrophes to escape.The command also infers
requirement_linkrows for new requirements: structural links from sub-issues and references as facts, content-based links asorigin='inferred',is_suspect=1, with a written rationale.4.
/qk-tickets— source selection + bug fixes. Takesmasterorpr <number>; with no argument it lists open sync PRs and asks. For a PR it overlays only the migration files onto the current checkout (no branch switch) and reverts afterwards. Adds parallel-work analysis: tickets are unsafe together if linked bydepends_on(incl. transitively),conflicts_with, or a parent/childrefines/derives_from.It was also already broken: the query selected
story_pointsandassigned_to, which are not columns inschema.sql, so it errored out entirely; and it documentedblocks/related_tolink types absent from the CHECK constraint. Now grouped by priority, using the real types.5.
requirements.dbuntracked..gitignorehas listedrequirements/*.dbsince #695, but the file was already tracked by then — it was swept into #772 (an unrelated list-types PR) the same day, and gitignore does not apply to tracked files. Verified a fresh clone rebuilds it from SQL alone, identical content (both dumps hash to07b2f180…).Status changes
The mapping now covers the whole board, not just Refined Tickets and beyond:
Backlog→draft,Prioritised Backlog→proposed— matching whatseed.sqlalready did for those columns. A ticket moving backwards is mirrored like any other move and recorded inrequirement_change. The old never-down-status rule existed to guard against guessed data; with a readable board it just held the DB out of date.157 of 159 pre-existing requirements are unchanged. Only two moved:
verifieddraftapprovedimplementedBoth recorded as
status_changedat version 2 with old/new values.schema.sqlis unchanged — no new status type.in_progressappears in the data for the first time, but it was always in the CHECK constraint and the old workflow already mappedIn Progress=in_progress; it simply never managed to write one.Other field changes on existing rows: 4
granularitybackfills (NULL →story, fromtype:*labels), 19impl_pathsrefreshes from merged-PR file lists. Noimpl_commit_shachanges.For a human to look at
draft. That's board hygiene, not a DB error — move [VIBE-156] CaTH Publication - Manual Publishing #213 to Done and the next sync restoresverified.implementedstatus. Someone should decide whether they belong on the board.is_suspect=1. Worth a sanity check before relying on the parallel-work grouping.Verification
yarn requirements:buildsucceeds;PRAGMA integrity_check→ok;PRAGMA foreign_key_check→ empty.fetch_board.shfailure paths tested: badPROJECT_IDand empty response each exit 1 with a clear cause and no output.yarn lintpasses across all 65 packages.@hmcts/court-of-appeal-civil-daily-cause-listfails withCannot find package '@hmcts/et-daily-list'. Confirmed identical on clean master with these changes stashed.Note for reviewers
After merging, run
yarn requirements:build—requirements.dbis no longer tracked, so a stale local copy won't be updated bygit pull.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores