Skip to content

Prioritize the main-to-develop sync resolver - #426

Merged
lopugit merged 3 commits into
github-actionsfrom
codex/lopu-sync-priority-421
Aug 27, 2026
Merged

Prioritize the main-to-develop sync resolver#426
lopugit merged 3 commits into
github-actionsfrom
codex/lopu-sync-priority-421

Conversation

@lopugit

@lopugit lopugit commented Aug 27, 2026

Copy link
Copy Markdown
Owner

Summary

  • dispatch the automation-owned sync/main-into-develop -> develop conflict lane before ordinary conflict batches
  • preserve canonical number-sorted, bounded batches by giving the sync lane its own first handoff
  • keep every other PR in deterministic numeric order on the same durable single-Lopu fleet queue
  • add controller contract coverage for priority partitioning, observability, and bounded canonical batches

Why

PR #421 was detected after the sync branch refreshed to the latest main, but a full scan placed it at item 20 of 21 behind historical promotion conflicts. This keeps repository plumbing responsive without cancelling active work or weakening snapshot validation.

Validation

  • node .github/scripts/resolve-pr-conflicts-routing-contract.mjs --self-test
  • node .github/scripts/workflow-control-plane-contract.mjs --self-test
  • node .github/scripts/promotion-worker-routing-contract.mjs --self-test
  • bash .github/scripts/promotion-worker-contract.sh
  • git diff --check
  • graphify update .
  • graphify cluster-only .
  • GRAPHIFY_VIZ_NODE_LIMIT=1000000 graphify export html

@github-actions

Copy link
Copy Markdown
Contributor

Lopu review — PR #426 (head d3de73e9 vs github-actions @ cb7e6f2a)

The routing change is correct and well-targeted. I verified the mechanism rather than taking the diff at its word, found one real regression in the companion graphify commit and fixed it in the worktree, and traced the red check to an external cause.

✅ The priority lane works, and for the right reasons

  • Predicate matches real data — the detector emits head/base from headRefName/baseRefName, so those fields are genuinely populated on every record reaching the handoff.
  • Branch name is exactsync-main-into-develop.yml publishes sync/main-into-develop with base=develop; live PR Sync main into develop #421 matches exactly.
  • The lane is reachablesync/main-into-develop is unprotected, so the detector's protected-head skip doesn't silently drop it first.
  • Nothing cancels anything — the worker group is resolve-worker-<run_id> (unique per run) with top-level cancel-in-progress: false. Both runs then join lopu-agent-fleet-<repo> (queue: max), and since the priority run is created first it enters that queue first. Single-fleet serialization is preserved.

Since the contracts only assert the text of the handoff step, I drove the extracted logic against fixtures:

Case Result
#421 scenario (sync PR + 5 ordinary) priority [421] first, then [99,150,222,301,410]TOTAL=6
head sync/main-into-develop, base main correctly stays in the ordinary lane (no over-match)
no sync PR single ordinary dispatch, identical in shape to the old behaviour
empty batch no dispatch, TOTAL=0, guard still errors
451 PRs priority [421], then bounded batches of 200 / 200 / 50 in numeric order

manual_retry survives partitioning. All four repo contracts pass, git diff --check is clean, and the YAML parses.

🔧 Fixed: d3de73e9 poisoned the tracked graph

The graphify index commit was generated locally without semantic extraction and degraded graphify-out/:

  • Node IDs were rewritten from a machine-local pathclaude_thingtime_ai_instructions (source_file: CLAUDE.md) became users_lopu_codex_worktrees_lopu_sync_priority_<suffix>_agents_…. Those IDs don't reproduce on any other machine, so every later graphify update would churn them again — guaranteeing repeated ~8k-line diffs and conflicts under graphify-out/, which is the exact workload this PR is trying to reduce.
  • The semantic layer for this PR's own file was deleted — all 9 concept nodes for resolve-pr-conflicts.yml (detector_handoff, conflict_resolution, graphify_refresh, workflow_call, the trigger/label nodes) are gone, because manifest.json recorded "semantic_hash": "" for both changed files and no cache/semantic/ entry was added — unlike the healthy prior index commit fa9fb680. Those are the nodes AI_ALL.md tells us to reach via graphify query / graphify explain.
  • Net: nodes 603 → 600, links 1280 → 1250. CI wouldn't have caught it — graph_not_collapsed only refuses a refresh that loses more than half the nodes.

To be precise: no absolute path appears as literal content, source_file stayed repo-relative. The damage is to the node-ID namespace and the missing concept layer.

I restored all three graphify-out/ files to their base contents, together, keeping the atomic pair AI_ALL.md requires. Regenerating here wasn't an option worth taking — graphify isn't on this runner and there's no semantic credential, so it would just re-poison the IDs with the runner's paths — while the control plane already rebuilds the graph after every merge with cache-served semantic extraction. A slightly stale but consistent pair beats a non-reproducible one that lost the resolver's concepts. The PR now carries only its intentional change: 2 files, +79/−43.

🟡 The red check isn't yours

control-plane / Lopu builds and repairs the all branch was cancelled, not failed — zero steps recorded, no log blob, ended while still queued. Across the last 30 Build all branch runs, 15 cancelled / 13 succeeded across many unrelated branches, with successes as recent as 01:19Z today. It's ordinary supersession in the shared fleet queue; #426 touches neither all-branch.yml nor build-all-branch.mjs. No control-plane repair was warranted.

CodeQL: the trusted snapshot for this head is empty — nothing to fix or dispose.

Two notes, neither blocking

  • The empty-batch guard moved after the dispatch loop. With empty input nothing dispatches, so the error and exit code are unchanged — only its position moved.
  • unique_by(.number) is now per-partition rather than global. That could only double-dispatch if PRS held two records with the same number but different head/base — it can't, since the scan step assigns prs="$(query)" from a single query. Noted for future readers.

Recommendation: merge as it now stands. Worth considering separately: a local graphify update . without a semantic credential silently produces this kind of degraded index, so it's safer to let the post-merge CI refresh own graphify-out/ than to hand-commit it from a worktree.

🤖 Lopu · automated repository review

@github-actions

Copy link
Copy Markdown
Contributor

Lopu repository review

Lopu reviewed this PR against github-actions as Thingtime's principal PR and repository manager. Using Claude Opus 5.

Lopu made justified improvements and pushed f13a411 to codex/lopu-sync-priority-421.

Lopu review — PR #426 · Prioritize the main-to-develop sync resolver

Compared: head d3de73e9 against base github-actions @ cb7e6f2a (full worktree diff, 5 files).
Verdict: the control-plane change is correct and well-targeted. One real regression in the
companion graphify index commit was found and fixed in the worktree. The one red check is not
caused by this PR.

What I compared

Finding 1 — Routing change is correct (no change needed)

The priority partition does what the PR description claims, and I verified each link in the chain
rather than reading it off the diff:

  • The predicate matches real data. The detector emits {number, head, base, mode, manual_retry, ...} with head/base from headRefName/baseRefName, so .head and .base
    are genuinely populated on every record reaching the handoff.
  • The branch name is exact. sync-main-into-develop.yml publishes SYNC_BRANCH: sync/main-into-develop and opens the PR with base=develop. Live PR Sync main into develop #421 is exactly
    sync/main-into-develop → develop.
  • The lane is reachable. sync/main-into-develop is unprotected, so the detector's
    protected-head skip does not silently drop it before the handoff.
  • Nothing cancels anything. The internal worker concurrency group is
    resolve-worker-<run_id> (unique per run) and top-level cancel-in-progress is false, so the
    second dispatch cannot cancel the first. Both worker runs then join the durable
    lopu-agent-fleet-<repo> queue (queue: max, cancel-in-progress: false), and because the
    priority run is created first it enters that queue first. That is precisely the mechanism the PR
    needs, and it preserves the single-fleet serialization.

Two smaller observations, both non-blocking and deliberately left alone:

  • The empty-batch guard moved from before the dispatch loop to after it. With an empty input no
    dispatch occurs, so the error and exit code are unchanged — only its position moved.
  • unique_by(.number) is now per-partition rather than global. This could only dispatch a PR twice
    if PRS ever held two records with the same number but different head/base. It cannot: the
    scan step assigns prs="$(query)" from a single query (reassigned, never concatenated), so there
    is one record per PR number. Noted for future readers, not a defect today.

Finding 2 — Fixed: the graphify index commit poisoned the tracked graph

Commit d3de73e9 ("chore(graphify): index sync conflict priority") was generated in a local
worktree without semantic extraction, and it degraded the tracked knowledge graph:

  • Node IDs were rewritten from a machine-local path. The canonical claude_* nodes
    (source_file: CLAUDE.md) were replaced by IDs slugified from the author's absolute worktree
    path, e.g. claude_thingtime_ai_instructions
    users_lopu_codex_worktrees_lopu_sync_priority_<suffix>_agents_thingtime_ai_instructions.
    These IDs are not reproducible on any other machine, so every later graphify update would churn
    them again — guaranteeing repeated ~8k-line diffs and merge conflicts under graphify-out/.
  • The semantic layer for this PR's own file was deleted. All 9 concept nodes for
    .github/workflows/resolve-pr-conflicts.yml (detector_handoff, conflict_resolution,
    graphify_refresh, workflow_call, the trigger and label nodes) were dropped, because semantic
    extraction did not run: manifest.json recorded "semantic_hash": "" for both changed files,
    and unlike the healthy prior index commit fa9fb680, no graphify-out/cache/semantic/ entry was
    added. Those are exactly the nodes AI_ALL.md tells agents to reach via graphify query /
    graphify explain.
  • Net loss: nodes 603 → 600, links 1280 → 1250.
  • CI would not have caught this: the graph_not_collapsed guard only refuses a refresh that loses
    more than half the nodes.

To be precise about scope: no absolute path appears as literal content in the tracked files —
source_file stayed repo-relative. The damage is to the node-ID namespace and the missing concept
layer, not a path string leak.

Change made: restored graphify-out/GRAPH_REPORT.md, graph.json, and manifest.json to
their base (cb7e6f2a) contents. All three were reverted together, keeping the atomic
graph/manifest pair that AI_ALL.md requires. This is the right resolution rather than
regenerating locally, because graphify is not installed on this runner and no semantic credential
is available here — regenerating would re-poison the IDs with this runner's paths — while the
control plane already rebuilds the graph after every merge with structural and cache-served
semantic extraction. A slightly stale but consistent pair is strictly better than a
non-reproducible one that has lost the resolver's concept nodes.

The PR now carries only its intentional change: 2 files, +79/−43.

Finding 3 — The red check is external, not a PR defect

control-plane / Lopu builds and repairs the all branch shows as failing. It was cancelled,
not failed: the job recorded zero steps, its log blob does not exist, and it ended 3m46s after
starting while still queued. all-branch.yml's rebuild job sits in the shared
lopu-agent-fleet-<repo> queue, and across the last 30 Build all branch runs 15 were cancelled
and 13 succeeded, spread over many unrelated branches (codex/batch-pr-conflict-resolutions,
claude/fallback-model-selection-0281b1, lopu/workflow-check-fix-*), with successes as recent as
today 01:19Z. This is ordinary queue supersession under PR load, not a reproducible defect and not
attributable to #426, which touches neither all-branch.yml nor build-all-branch.mjs.

No control-plane repair was warranted, so $RUNNER_TEMP/lopu-workflow-fix.md was not created and
nothing in $GITHUB_WORKSPACE/trusted was modified.

CodeQL

The trusted snapshot for this head is empty ([]). There was nothing to fix and nothing to
dispose; the disposition file was left as [].

Validation run

Check Result
node .github/scripts/resolve-pr-conflicts-routing-contract.mjs --self-test PASS
node .github/scripts/workflow-control-plane-contract.mjs --self-test PASS
node .github/scripts/promotion-worker-routing-contract.mjs --self-test PASS
bash .github/scripts/promotion-worker-contract.sh PASS
git diff --check clean
resolve-pr-conflicts.yml YAML parse valid
graphify pair after revert 603 nodes · 13 claude_* · 9 resolver concept nodes · 0 machine-path nodes

The repository's contracts only assert the text of the handoff step, so I additionally drove the
extracted partition logic against fixtures to test its behaviour:

  • Sync main into develop #421 scenario (sync PR mixed into 5 ordinary PRs): priority batch [421] dispatched first,
    then one ordinary batch [99,150,222,301,410]. TOTAL=6, nothing dropped.
  • Narrowness: a PR with head sync/main-into-develop but base main correctly stayed in the
    ordinary lane — the predicate does not over-match.
  • No-sync-PR regression: a single ordinary dispatch, byte-identical in shape to the old
    behaviour.
  • Empty batch: no dispatch, TOTAL=0, so the guard still errors.
  • Scale: 451 PRs → priority [421], then bounded ordinary batches of 200 / 200 / 50 in strict
    numeric order.
  • manual_retry survives partitioning unchanged.

Recommendation

The routing change is sound and I would merge it as it now stands. The only follow-up worth
considering separately: local graphify update . without a semantic credential silently produces
this kind of degraded index, so it is safer to let the post-merge CI refresh own graphify-out/
than to hand-commit it from a worktree.

View Lopu workflow run

@lopugit
lopugit merged commit 3d5e7df into github-actions Aug 27, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant