Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion .github/workflows/ci-maintenance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: CI Maintenance
# Trusted base-branch code only; never checks out or executes PR content.
on:
pull_request_target:
types: [closed, synchronize]
types: [closed, synchronize, opened, reopened, edited]

permissions:
contents: read
Expand All @@ -13,6 +13,51 @@ concurrency:
cancel-in-progress: true

jobs:
refresh-pr-diff:
if: github.event_name == 'pull_request_target' && github.event.action != 'closed'
runs-on: ubuntu-latest
timeout-minutes: 2
permissions:
contents: read
pull-requests: write
steps:
- uses: actions/github-script@v8
with:
script: |
const repo = context.repo;
const pull_number = context.payload.number;
const expected = context.payload.pull_request;
const readPR = async () => (await github.rest.pulls.get({...repo, pull_number})).data;
const pr = await readPR();
const sameIdentity = current => current.state === 'open' &&
current.head.sha === expected.head.sha && current.head.ref === expected.head.ref &&
current.head.repo?.id === expected.head.repo?.id &&
current.base.ref === expected.base.ref && current.base.repo?.id === expected.base.repo?.id;
if (!sameIdentity(pr) || !pr.head.repo?.id || !pr.base.repo?.id) {
return core.info('PR identity changed; stop diff refresh.');
}
const readTarget = async () => (await github.rest.repos.getBranch(
{...repo, branch: pr.base.ref})).data.commit.sha;
const target = await readTarget();
if (pr.base.sha === target) return core.info('PR base snapshot is current.');
// GitHub can retain the original base snapshot after main is merged
// into the source branch, counting those unrelated changes again.
// Re-select the same base; never rewrite the source or retarget it.
const before = await readPR();
if (!sameIdentity(before) || before.base.sha !== pr.base.sha) {
return core.info('PR changed before update; stop diff refresh.');
}
await github.rest.pulls.update({...repo, pull_number, base: pr.base.ref});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve a concurrent target edit before applying the stale base

在最后一次 readPR 后、PATCH 真正应用前,维护者仍可把目标从 main 改到 release/stable。这里发送的 base 仍为旧 main;该 REST 字段会直接设置目标,因此会覆盖这次合法修改。之后 sameIdentity(after) 又匹配旧事件,任务甚至报告 Refreshed。原样执行这段 JavaScript,按文档实现 params.base 赋值并在写前插入 retarget 即可复现;baseline 无该任务时新目标保留。现有 base_during_write mock 只在更新后改 ref,没有模拟 PATCH 覆盖它。请使 provider 写入具备真正的目标条件/协调,或先将自动路径保持只读,并增加这个最后 GET 与实际 PATCH 之间的反例;再多一次 GET 仍有相同窗口。

const after = await readPR();
if (!sameIdentity(after)) return core.info('PR changed during refresh; inspect the newer revision.');
// A concurrent target push may have been included by the update.
if (after.base.sha !== target && after.base.sha !== await readTarget()) {
throw new Error('PR base snapshot did not refresh; inspect its diff.');
}
core.info(`Refreshed #${pull_number} at unchanged head ${after.head.sha}: ` +
`${pr.changed_files} -> ${after.changed_files} files, ` +
`+${pr.additions}/-${pr.deletions} -> +${after.additions}/-${after.deletions}.`);

cancel-obsolete-pr:
if: github.event_name == 'pull_request_target' && (github.event.action == 'closed' || github.event.action == 'synchronize')
runs-on: ubuntu-latest
Expand Down
33 changes: 32 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ touch, mention it in the PR instead of fixing it there.
| --- | --- | --- | --- |
| `python-tests.yml` | every PR | yes (`merge-gate`) | lint, mypy, sharded pytest, TypeScript core and coverage, minimum Node.js, Windows PowerShell, dashboard presentation |
| `dco.yml` | every PR | yes (`Sign-off`) | `Signed-off-by` on contribution commits; verified GitHub-generated two-parent merges are exempt |
| `ci-maintenance.yml` | closed or updated PR (`pull_request_target`, trusted base code only) | no | cancels obsolete Python qualification runs after rechecking the PR and source branch |
| `ci-maintenance.yml` | opened, reopened, edited, updated or closed PR (`pull_request_target`, trusted base code only) | no | refreshes stale PR diff bases without changing the head; cancels obsolete Python qualification runs after rechecking the PR and source branch |
| `dependency-review.yml` | every PR | no | dependency changes introduced by the PR |
| `postgresql-integration.yml` | control-plane or npm lockfile paths | no | PostgreSQL authority store and service on a temporary instance |
| `package-smoke.yml` | extension package paths | no | extension packages install, entrypoints, and example schemas |
Expand Down Expand Up @@ -380,6 +380,37 @@ Maintainers may request consolidation when a useful outcome was unnecessarily
split, or a smaller PR when unrelated concerns were mixed. Review evaluates the
verified goal delta and evidence, not minimum size, model identity or PR count.

### Verify the published diff

After opening or updating a PR, fetch its actual target branch and source head,
then compare GitHub's changed-file inventory and line totals with
`git diff --stat TARGET...HEAD` and `git diff --name-status TARGET...HEAD` at those
exact revisions. Use the actual PR target, including a non-main target; a
two-dot comparison with an advancing target also includes target-only changes.

GitHub's [PR and compare views can use different merge bases](https://docs.github.com/en/pull-requests/reference/pull-requests#differences-between-commits-on-compare-and-pull-request-pages).
The PR can retain an older base snapshot after the source branch incorporates
target commits, inflating both files and line totals. CI Maintenance refreshes
that snapshot on PR lifecycle events, using trusted inline code without checking
out the contributor's branch. It re-selects the same base, verifies the source
identity before writing and reads back the unchanged head and refreshed base.
It does not repair unrelated code that is genuinely present in the Git diff.

If published totals still disagree, inspect the PR's exact repository, head and
base, and compare the pinned base with the current target. A maintainer can
refresh the same base with the native API:

```bash
gh api --method PATCH repos/OWNER/REPO/pulls/NUMBER -f base=VERIFIED_BASE_BRANCH
```

Read back the unchanged head, unchanged target branch and corrected file/line
inventory before declaring the repair complete. Recheck moved revisions rather
than reusing stale evidence. A refresh needs no empty commit, history rewrite,
temporary retarget or close/reopen cycle. Ordinary target advancement alone is
not evidence of an incorrect diff. If the Git diff itself contains unrelated
work, isolate the intended changes on the verified target before publishing.

### Validation disclosure

Use the [PR template](.github/PULL_REQUEST_TEMPLATE.md) to report facts, not a
Expand Down
1 change: 1 addition & 0 deletions skills/loopx-self-repair/references/repair-patterns.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ teaches a reusable control-plane lesson.
| `todo_succession_gap` | A non-trivial feature todo is marked done after PR/smoke success, but rollout, product-path audit, docs, telemetry, benchmark proof, or operator-decision follow-up is only mentioned in prose or not recorded at all. | completed todo note/evidence, recent PR/run history, active `Agent Todo`, active `Next Action`, related design docs. | Agent treated an implementation slice as feature completion, or LoopX lacked a reminder to create the successor todo. | Keep the lifecycle simple: do not add many feature states. Complete the old todo and create the next concrete agent/user todo with `--next-agent-todo`/`todo add`, or record a compact no-follow-up rationale. |
| `todo_parent_successor_frontier_gap` | An open parent remains quota-runnable after its work was split into explicit successors, while every unfinished successor is deferred or owned by another peer and the parent has no independent immediate action. | parent status and `successor_todo_ids`, successor ownership/resume conditions, `todo update` response `parent_successor_advisory`, current quota candidate. | Successor links were mistaken for suspension or aggregate lifecycle state even though they intentionally encode lineage only. | Do not infer aggregation from successor links. At the authoring boundary, require the agent to decide whether the parent retains an independent immediate action; if not, explicitly defer it with a supported `resume_when` or complete it. Keep the reminder advisory and machine-readable rather than adding persisted aggregate state. |
| `integration_remote_head_observation_gap` | `integration-branch status/sync --refresh-remotes` reports `in_sync`, but a normal push is rejected because another agent advanced the published integration branch. | integration plan, configured branch upstream, local and remote integration SHAs, merge-base relation, normal push result. | Remote refresh observed only base/source inputs, while tree-equivalent local candidates were treated as sufficient even when they omitted the published integration ancestry. | Narrow-fetch the configured integration upstream with base/source refs, surface remote-ahead or diverged ancestry as typed drift, and require automatic or supplied candidates to contain that exact remote head before recording an in-sync receipt. Never force-push or prune unrelated refs. |
| `github_pr_stale_diff_base` | GitHub PR files, line totals or commit count include target-branch changes absent from the current three-dot Git diff, often after merging the target into the source branch. | Exact source repository/head, actual target branch/tip, PR pinned base, both merge bases, complete changed-file inventories and line totals before/after. | GitHub retained an older PR base snapshot; checking only the source head or comparing two tips missed the stale comparison. | Prove the discrepancy against the current target merge base, then re-select the same verified base through the native PR API under existing write authority. Read back unchanged head/target and corrected inventory; do not force-push, add an empty commit or temporarily retarget. Check all requested open PRs, preserve genuine source changes, and cover lifecycle refresh in trusted CI Maintenance plus published-diff readback in CONTRIBUTING.md. |
| `plan_todo_writeback_gap` | The agent explains a route change, top-20 todo list, P0/P1 plan, or deprecation policy to the user, but the active LoopX todos / Next Action / refresh-state payload do not contain the same commitments. | latest user-facing plan, quota `todo_write_hint`, active `Agent Todo`, `User Todo`, `Next Action`, recent refresh-state runs. | Agent used chat as memory after understanding the plan; LoopX prompt/skill did not make plan publication a writeback trigger. | Treat user-facing LoopX plans as writeback triggers: add/update concrete todos or refresh state before final, or explicitly mark the plan speculative/no-writeback with rationale. Add skill/prompt wording when this recurs. |
| `gated_unmerged_change_continuity_gap` | Work is prepared, staged, or committed locally while a repository change window blocks commit or push, but the durable state records only a future reminder; work that never attempts a Git operation is especially easy to miss. After restart or handoff, no global surface can identify the producing repository, checkout/head, worktree, changed-path inventory, validation, or terminal outcome. | change-window decision, pending-change global projection, private locator health and path count, repository checkout/head and worktree fingerprint, linked Todo, validation and PR refs, plus an explicit linked-worktree reconciliation preview when broad recovery is requested. | Schedule enforcement and work continuity were modeled as separate prose conventions, and hook denial was the only enrollment seam, so correctly paused work could silently orphan the code needed after the gate reopened. | On a denied repository hook, append or idempotently refresh a public-safe pending-change event. At bounded writeback or handoff during a blocked window, reconcile the current dirty checkout even when no Git operation was attempted; keep a broad linked-worktree sweep explicit and treat unattributed historical dirt as candidates rather than proof of current-gate causality. Store repository identity, branch or detached checkout/head, content-free fingerprint, next eligible time, and counts in the shared projection; retain the worktree and repository-relative changed paths only in a mode-0600 private locator. Allow explicit goal/Todo/write-scope/validation enrichment, expose cross-project list/verify, and require typed merged/superseded/abandoned resolution. Never copy code, diff bodies, credentials, file names, or personal paths into the shared projection. |
| `capability_route_hidden_in_prompt` | A caller explicitly selects a registered capability, but `start-goal` exposes the route only in planning prose; or the first turn qualifies correctly while a later heartbeat loses the route and writes a plausible generic implementation Todo. | exact start-goal route switch, full and compact packets, guided `ordered_steps`, capability admission state, projected successor Todo, later quota `selected_todo`. | Capability selection remained optional prose, or a bootstrap-only route was mistaken for durable later-turn authority. | Project the explicit switch as a typed bootstrap route and pre-implementation guard, persist admission in capability-owned state, then write its exact generic successor with `action_kind` plus a stable public `target_key`. Preserve those scheduling fields in later quota selection; do not copy domain facts into Todo or infer admission from stale prompt context. Cover switch-off/on, compact/full parity, exact successor validation, and later-turn quota re-entry. |
Expand Down
104 changes: 104 additions & 0 deletions tests/test_ci_resource_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,110 @@ def test_cleanup_concurrency_cannot_replace_current_qualification() -> None:
assert qualification != cleanup


@pytest.mark.parametrize("action", ["opened", "reopened", "edited", "synchronize", "closed"])
def test_diff_refresh_uses_trusted_code_and_its_own_minimal_write_scope(action: str) -> None:
policy = workflow("ci-maintenance")
job = policy["jobs"]["refresh-pr-diff"]
context = event("pull_request_target")
context["github"].event.action = action
assert action in policy[True]["pull_request_target"]["types"]
assert expression(job["if"], context) is (action != "closed")
assert policy["permissions"] == {"contents": "read"}
assert job["permissions"] == {"contents": "read", "pull-requests": "write"}
assert job["timeout-minutes"] <= 2
assert all(step["uses"].startswith("actions/github-script@") for step in job["steps"])


def _run_diff_refresh(mode: str, branch: str = "main") -> dict:
script = workflow("ci-maintenance")["jobs"]["refresh-pr-diff"]["steps"][0]["with"]["script"]
harness = r'''
let input = ''; for await (const chunk of process.stdin) input += chunk;
const {script, mode, branch} = JSON.parse(input);
const oldBase = 'a'.repeat(40), head = 'b'.repeat(40), target = 'c'.repeat(40), newer = 'd'.repeat(40);
const pr = {state:'open', head:{sha:head,ref:'work',repo:{id:2}},
base:{sha:oldBase,ref:branch,repo:{id:1}}, changed_files:9,additions:30,deletions:12};
const expected = structuredClone(pr);
if (mode === 'closed') pr.state = 'closed';
if (mode === 'fresh') pr.base.sha = target;
if (mode === 'old_event_head') expected.head.sha = newer;
if (mode === 'old_event_source') expected.head.repo.id = 3;
if (mode === 'old_event_base') expected.base.ref = 'other';
if (mode === 'missing_source') pr.head.repo = null;
if (mode === 'same_repo') {pr.head.repo.id = 1; expected.head.repo.id = 1;}
let reads = 0, branchReads = 0;
const updates = [], notes = [];
const context = {repo:{owner:'owner',repo:'fixture'}, payload:{number:17,pull_request:expected}};
const github = {rest:{pulls:{get:async()=>{
reads++;
if (reads === 2) {
if (mode === 'head_before_write') pr.head.sha = newer;
if (mode === 'source_before_write') pr.head.repo.id = 3;
if (mode === 'base_before_write') pr.base.ref = 'other';
if (mode === 'refreshed_before_write') pr.base.sha = target;
}
return {data:structuredClone(pr)};
}, update:async params=>{
updates.push(params);
if (mode === 'failed_api') throw new Error('provider rejected update');
if (mode !== 'not_applied') pr.base.sha = mode === 'target_moves' ? newer : target;
if (mode === 'head_during_write') pr.head.sha = newer;
if (mode === 'base_during_write') pr.base.ref = 'other';
pr.changed_files = 2; pr.additions = 3; pr.deletions = 1;
}}, repos:{getBranch:async params=>{
if (params.branch !== branch) throw new Error('unverified target');
branchReads++;
return {data:{commit:{sha:mode === 'target_moves' && branchReads > 1 ? newer : target}}};
}}}};
let error = null;
try {await new (Object.getPrototypeOf(async function(){}).constructor)('github','context','core',script)
(github,context,{info:message=>notes.push(message)});} catch(e) {error=e.message;}
process.stdout.write(JSON.stringify({updates,notes,error,pr}));
'''
result = subprocess.run(
["node", "--input-type=module", "-e", harness],
input=json.dumps({"script": script, "mode": mode, "branch": branch}),
text=True, capture_output=True, check=True, timeout=10,
)
return json.loads(result.stdout)


@pytest.mark.parametrize("branch", ["main", "release/stable"])
@pytest.mark.parametrize("mode", ["stale", "same_repo", "target_moves"])
def test_stale_diff_refresh_preserves_the_exact_source_and_actual_target(mode: str, branch: str) -> None:
observed = _run_diff_refresh(mode, branch)
assert observed["error"] is None
assert observed["updates"] == [{"owner": "owner", "repo": "fixture", "pull_number": 17, "base": branch}]
assert observed["pr"]["head"] == {
"sha": "b" * 40, "ref": "work", "repo": {"id": 1 if mode == "same_repo" else 2},
}
assert observed["pr"]["base"]["ref"] == branch
assert any("9 -> 2 files" in note and "+30/-12 -> +3/-1" in note for note in observed["notes"])


@pytest.mark.parametrize("mode", [
"fresh", "closed", "old_event_head", "old_event_source", "old_event_base", "missing_source",
"head_before_write", "source_before_write", "base_before_write", "refreshed_before_write",
])
def test_diff_refresh_does_not_write_after_identity_or_snapshot_drift(mode: str) -> None:
observed = _run_diff_refresh(mode)
assert observed["updates"] == []
assert observed["error"] is None


@pytest.mark.parametrize("mode", ["head_during_write", "base_during_write", "not_applied", "failed_api"])
def test_diff_refresh_requires_readback_and_never_retries_a_moved_revision(mode: str) -> None:
observed = _run_diff_refresh(mode)
assert len(observed["updates"]) == 1
assert not any("Refreshed #" in note for note in observed["notes"])
if mode == "not_applied":
assert "did not refresh" in observed["error"]
elif mode == "failed_api":
assert "provider rejected update" in observed["error"]
else:
assert observed["error"] is None
assert any("newer revision" in note for note in observed["notes"])


@pytest.mark.parametrize("mode", ["delayed", "never", "changed"])
def test_obsolete_run_cancellation_waits_for_verified_completion(mode: str) -> None:
script = workflow("ci-maintenance")["jobs"]["cancel-obsolete-pr"]["steps"][0]["with"]["script"]
Expand Down
4 changes: 3 additions & 1 deletion tests/test_sonarcloud_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,9 @@ def test_separate_trusted_cleanup_does_not_activate_analysis() -> None:
caller = yaml.load(WORKFLOW.with_name("python-tests.yml").read_text(), Loader=yaml.BaseLoader)
maintenance = yaml.load(WORKFLOW.with_name("ci-maintenance.yml").read_text(), Loader=yaml.BaseLoader)
assert "pull_request_target" not in caller["on"]
assert maintenance["on"]["pull_request_target"]["types"] == ["closed", "synchronize"]
assert set(maintenance["on"]["pull_request_target"]["types"]) == {
"closed", "synchronize", "opened", "reopened", "edited",
}
for job in maintenance["jobs"].values():
assert "uses" not in job and "secrets" not in job
assert all(not step.get("uses", "").startswith("actions/checkout@") for step in job["steps"])
Expand Down
Loading