Repository navigation
Conversation
Updates the requirements on [sentence-transformers](https://github.com/huggingface/sentence-transformers) to permit the latest version. - [Release notes](https://github.com/huggingface/sentence-transformers/releases) - [Commits](huggingface/sentence-transformers@v3.0.0...v6.0.0) --- updated-dependencies: - dependency-name: sentence-transformers dependency-version: 6.0.0 dependency-type: direct:development ... Signed-off-by: dependabot[bot] <support@github.com>
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #872
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #873 Maintainer corrections and scope extraction applied where needed.
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #875 Maintainer corrections and scope extraction applied where needed.
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #955
…int resolution Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #858 Maintainer corrections and scope extraction applied where needed.
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #860 Maintainer corrections and scope extraction applied where needed.
…l_files Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #870
…t of the registry Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #915 Maintainer corrections and scope extraction applied where needed.
… an indexed tail lookup Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #942 Maintainer corrections and scope extraction applied where needed.
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #952
…stored paths Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #962
…andidates Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #966 Maintainer corrections and scope extraction applied where needed.
…n references_to Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #969 Maintainer corrections and scope extraction applied where needed.
…ctor workflows Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #821 Maintainer corrections and scope extraction applied where needed.
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #847 Maintainer corrections and scope extraction applied where needed.
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #874 Maintainer corrections and scope extraction applied where needed.
…em locale Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #928
Port the reviewed token-efficiency implementation with targeted regression coverage. Source-PR: #959
Visualization: correct the #609 numbers and the 2000-node auto-collapse threshold, describe the test suite accurately, decide the renderer/cap interaction (Python caps, client-side top-N fallback), specify the inline CSP (per-block hashes, worker-src blob:), move the page script into a shipped asset in Phase 2, pin --include/--exclude semantics and the _slim_for_page step, and pick Starlette under uvicorn for the Phase 4 server so LoopbackOriginGuard is reused. Multi-repo: name the resolvers that actually leave the root (_do_resolve_module branches, both TsconfigResolver methods), describe PR #880 correctly, use real query_graph patterns, add GraphStore.open_read_only, state where sibling maps are computed and how they reach process-pool workers, give impact radius a concrete mechanism, define the caveats key and budget entries, and record the bare-specifier behaviour change for ungrouped users. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Resolves conflicts in CONTRIBUTING.md, README.md, docs/GITHUB_ACTION.md and docs/INDEX.md by keeping this branch's rewritten docs and adding the staging/testing/main branching content from #989.
incremental_update matched caller-supplied changed_files against the ignore patterns as given. Absolute inputs under a root such as /tmp/... matched the anchored default pattern /tmp/** by their filesystem spelling, so on Linux CI the new watch/identity tests indexed nothing. Normalise absolute inputs that lie inside the repository to repo-relative POSIX paths first; paths outside the root are left to stale-file reconciliation as before. Also give the two new git-based tests an explicit user.name/user.email like the rest of the suite, so they run on a bare CI runner. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Every job reinstalled ~100 packages from PyPI. setup-python's built-in pip cache, keyed on pyproject.toml, restores the wheel cache instead. Caches are scoped per branch by GitHub (fork PRs cannot write caches the base branch reads), and the store is bounded and auto-evicted after 7 days unused. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ardening Integrate token-efficiency hardening sweep (20 contributor PRs + ~25 issue fixes)
Author
code-review-graph reviewOverall risk: 0.85 (CRITICAL) — 500 changed function(s)/class(es), 74 affected flow(s), 186 test gap(s) Risk-scored changes
Affected execution flows
Test gaps
Token savings: this graph-backed report used ~935,650 fewer tokens (~90%) than reading every changed file in full (estimated, chars/4 approximation).
Powered by code-review-graph — local-first analysis; no code leaves the CI runner. |
ci: cache pip downloads between runs
Every existing test calls the tool functions in-process. Nothing covered the path an actual editor takes: spawn `python -m code_review_graph serve`, speak MCP JSON-RPC over stdio, and read tool payloads back. A break in CLI argument wiring, tool registration, schema generation, payload serialisation or error propagation would have shipped unnoticed. tests/test_e2e_mcp_client.py builds a real git repository in tmp_path (explicit `-c user.name` / `-c user.email`, so it does not depend on the runner's global git config) with cross-file calls and a test module, then drives the server with `mcp.client.stdio` + `mcp.ClientSession`: initialize -> list_tools -> build_or_update_graph_tool (full) -> get_minimal_context_tool -> semantic_search_nodes_tool -> query_graph_tool(callers_of) x2 -> edit + commit -> build_or_update_graph_tool (incremental) -> detect_changes_tool -> list_repos_tool Assertions are on returned content — 3 files parsed, `compute_total` and `test_compute_total_sums_items` found, `normalize_amount` has exactly one caller joined by a CALLS edge, `build_invoice` resolves across the import, `changed_files == ["pkg/core.py"]` with a non-zero risk score — not merely on the absence of an error. A negative test confirms a repo_root outside a project root is refused by `_validate_repo_root` and the refusal reaches the client as an error, and a plain-subprocess test confirms `daemon status` exits 0 and reports no daemon against an isolated CRG_HOME. Note: `python -m code_review_graph.cli` is a no-op (cli.py has no `__main__` guard), so the runnable entry point is `-m code_review_graph`. stdio is the default transport, so there is no `--stdio` flag. Portability: pathlib throughout, `os.environ` copies (never mutated), no shell=True, CRLF-normalised text assertions, every client call and subprocess bounded at 120s, and clean skips when `mcp` or git is missing. CI gains a non-required `e2e` job on ubuntu/macOS/windows (Python 3.12, `pytest -m e2e -q`). The `test` job is unchanged: the three e2e tests add ~4s, which does not justify excluding them from the coverage matrix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every existing build test parses a few synthetic files in tmp_path. Those pin individual parser rules but cannot catch the failures that only show up at repository scale: a resolver that drops every edge, a postprocess stage that crashes on a real call graph, an incremental update that re-parses the world when nothing changed. tests/test_e2e_real_repo.py drives full_build -> trace_flows / detect_communities / rebuild_fts_index -> queries over the checkout the tests run from, and asserts facts that must hold of the real code: incremental.incremental_update and graph.GraphStore exist at the right file paths, incremental_update CALLS _should_ignore, tests/test_graph.py has a resolved IMPORTS_FROM edge to code_review_graph/graph.py, tests_for (GraphStore) reaches tests/test_graph.py, the impact radius of graph.py reaches its importers but not its own dependencies, communities partition the graph without double-booking a node, cli.py::main is traced as a flow entry point, hybrid search for "impact radius" ranks the implementation in the top 10, no node name carries a control character, get_stats matches the tables it summarises, and a second update over an unchanged tree is a no-op. A second pass builds tests/fixtures as its own repository root: every file the collector accepts produces at least one node, and Python, TypeScript, Go, Java, C#, Rust, Ruby, PHP and Kotlin each yield a CALLS or IMPORTS_FROM edge through a real build rather than a direct CodeParser call. Two substitutions against the original brief, because the literal assertions are not true of this code: code_review_graph/incremental.py has no IMPORTS_FROM edge to code_review_graph/graph.py, and the impact radius of graph.py therefore never reaches incremental.py. _extract_import reads the first dotted_name child of an import_from_statement, which for "from .graph import GraphStore" is the imported symbol, not the module, so the package's own relative imports never resolve to a file. The module asserts the equivalent facts that are true (tests/test_graph.py imports the package absolutely and does resolve) and documents the gap in place. Nothing is written into the checkout: the graph lives in a temp directory, CRG_HOME is redirected, and CRG_PARSE_EXECUTOR=thread keeps the parallel parse path under test without spawning process-pool workers that re-import the pytest session. The module is marked e2e (registered in pyproject) and skips cleanly outside a git checkout. It runs in ~7s locally, so it needs no slow marker and stays in the default CI run. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs(specs): visualization overhaul and multi-repo organisation designs
…ormers-gte-3.0.0-and-lt-7 deps: update sentence-transformers requirement from <6,>=3.0.0 to >=3.0.0,<7
tests/test_visualization.py asserts on the *text* of the HTML that
generate_html writes. That catches template regressions but not runtime
ones: markup containing d3.select("#graph-svg") can still paint nothing
because D3 failed to load, a handler threw, or a selector matched the
wrong element. Issue #523 was exactly that shape.
Add tests/test_visualization_browser.py: a 30-node / 30-edge graph across
3 files with 3 communities and one flow, generated in both auto (-> full)
and community mode, then driven in headless Chromium via Playwright's
sync API. It asserts the rendered node and edge counts under #graph-svg,
that the search box filters to a known symbol, that clicking a File node
collapses and re-expands the symbols it CONTAINS, that toggling an edge
type in the legend removes those lines, that the aggregated page paints
one super-node per community and drills in on double-click, and that no
test produces a console error or an uncaught page error.
Determinism notes:
- The page is served from a local http.server on 127.0.0.1:0, which is
what `visualize --serve` does. Every page also runs behind a page.route
guard that aborts any request leaving that origin, so no test can
silently depend on the network.
- Waits key off state the page exposes - #loading-overlay.hidden, set by
simulation.on("end"), and D3's own alpha < alphaMin on the aggregated
template, which has no overlay. Nothing sleeps.
- Before a mouse click the layout is frozen: every node is pinned to a
grid in the area the fixed panels leave clear and the zoom transform is
reset to the identity, so clicks are real mouse events with Playwright's
full actionability checks rather than a race against the simulation.
- The module is pytest.importorskip("playwright") and the browser fixture
skips cleanly when Chromium has not been installed.
Also documents a real limitation found while writing these: Chromium gives
every file:// document an opaque origin, so the SRI-pinned same-origin
<script src="d3.v7.min.js" integrity="..."> is blocked and a file:// open
falls back to the d3js.org CDN - i.e. double-clicking graph.html needs the
network, while `visualize --serve` does not. test_file_url_open_needs_the_cdn
pins that behaviour so a fix is noticed.
Packaging and CI:
- New "browser-test" extra (playwright, pytest-playwright), deliberately
not in [all] - it is contributor tooling and the tests additionally need
a ~150 MB Chromium fetched out of band.
- test_pr856_edges gains CONTRIBUTOR_EXTRAS so [all] must still cover every
runtime group while excluding dev and browser-test, plus a test asserting
[all] never pulls them in.
- Register the "browser" marker; new "viz-browser" CI job on ubuntu-latest
/ Python 3.12 runs `pytest -m browser -q` with no coverage flag, and the
coverage "test" job now runs with -m "not browser".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test(e2e): drive the MCP server through a real stdio MCP client
`from .registry import Registry` produced an IMPORTS_FROM edge to
`<repo>/code_review_graph/Registry.py` — a file that exists on no
case-sensitive filesystem. `from .cli import main` produced
`<repo>/code_review_graph/main.py`: a real file, and the wrong one.
0 of 222 relative imports in this package had a correct target.
tree-sitter-python wraps the module half of a relative import in a
`relative_import` node, never a `dotted_name`; the `dotted_name`
children that follow `import` are the imported SYMBOLS. Both
`_extract_import` and `_collect_import_names` scanned for the first
`dotted_name`, so both read a symbol as the module and discarded the
leading dots that set the package level. `from .m import a as b` and
`from .m import *` matched neither branch and emitted nothing at all.
Both now read `child_by_field_name("module_name")` — correct for the
relative and the absolute form alike — and relative modules resolve
through `_resolve_python_module_in_repo`, which already honoured the
level and refused to leave the repository and which
`_expand_python_star_imports` was already using two hundred lines away.
`from . import incremental` and `from .tools import query` additionally
emit an edge to the submodule they name; `from .graph import A, B`
stays one edge, so edge counts do not inflate.
`_collect_import_names` stores the resolved file for a relative import
(the form `_resolve_imported_symbol` and star-import expansion already
accept), which is what makes cross-module CALLS resolve.
Alongside: the absolute walk-up now stops at the repository root instead
of the filesystem root, and Python module probing compares against the
real directory listing, so a case-insensitive filesystem can no longer
answer "yes" to a spelling that does not exist.
Measured with scripts/audit_python_imports.py, which takes its ground
truth from CPython's own `ast` rather than from the graph:
this repo flask
relative imports correct 0/222 -> 222/222 0/167 -> 167/167
cross-module CALLS resolved 0/37 -> 37/37 11/67 -> 43/67
python import edges naming a file 37.1% -> 50.5% 19.7% -> 43.0%
edges targeting no node 40.2% -> 38.0% 46.2% -> 43.3%
targets naming a missing file 7 -> 0 17 -> 0
build wall time (median of 3) 3.64s -> 3.66s 0.87s -> 0.88s
tests/test_import_target_integrity.py is the CI guard: it builds a real
graph over a repository laid out with every form that used to break and
fails if any resolved target names a file that does not exist, compared
case-exactly so the guard bites on macOS as well as on Linux.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`forget X` must leave the graph a build without X would have produced.
It did not: `_collect_import_names` and `_expand_python_star_imports`
hand a relative specifier straight to `_resolve_python_module_in_repo`,
reaching around the `exclude_files` filter that only
`_resolve_module_to_file` applies. The re-parsed referrer rediscovered
the forgotten module on disk and kept a CALLS edge into a node `forget`
had just deleted. `tests/test_forget_parity.py` exercised only
`from util import helper` -- the absolute path, which was never broken.
Exclusions now apply inside `_python_file_exists`, so an excluded file
is missing where the probe happens rather than filtered one layer up.
Resolution then falls through to the next candidate exactly as the real
absence would, and every Python-specific caller inherits it. The
process-wide star-export cache is bypassed while exclusions are in
force, since its keys describe only the module's own mtime and size.
Also fixed, each with a test:
* Module/package collision. `pkg/m/__init__.py` beat by `pkg/m.py`
because `.py` was probed first. CPython's `FileFinder` checks the
package directory before any file loader, so the package wins;
verified with `PathFinder.find_spec`, not taken from the audit
script -- whose own `_module_file` had the same order and is
corrected here too. Same order applied to the absolute walk-up.
* PEP 420 namespace packages. `from .ns import leaf` with no
`pkg/ns/__init__.py` emitted the raw `.ns` specifier. The submodules
now resolve to their files; the namespace package itself contributes
no target, because it has no file for one to name.
* `from . import sub` emitted `REFERENCES` to `<package __init__>::sub`,
matching no node. A name bound to a module is marked as such in the
import map and resolves to the module's file node.
* `_extract_import` and `_collect_import_names` disagreed about
`from . import cli`: the edge named `pkg/cli.py`, the import map named
`pkg/__init__.py`. Both now name the submodule file.
* Package re-exports. `from .tools import query_graph` qualified the
symbol against `tools/__init__.py`, which does not define it. The
package's export map is followed to the defining file.
Measured on this repository (full build, post-processed):
relative imports correct 222/222 unchanged
cross-module CALLS resolved 37/37 unchanged
path-shaped CALLS targets
matching no node 239 -> 105 (155 before the PR)
of those, into __init__.py 130 -> 0 (73 before the PR)
resolved CALLS endpoints 12055 -> 12146
build wall clock, median of 5 6.42s -> 6.71s
The four functions the review flagged as losing `callers_of` keep
losing them, and should: every one of those 61 edges was a
misattribution the widened import evidence correctly withdrew --
`subprocess.run` counted as `agent_baseline.run` (44 of the 61),
`thread.start`/`observer.stop` as `GraphUpdateHandler.start`/`.stop`,
`store.get_impact_radius` as `tools/query.get_impact_radius`.
pallets/flask (`--depth 30`): 167/167 relative imports, unchanged.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…'s imports
`_resolve_bare_endpoints` resolved a bare CALLS target by name as soon as the
call-site file imported one same-named node. A receiver was never consulted,
so `some_dict.get(...)` was recorded as a call into `ConnectionPool.get`. The
bug predates this branch, and this branch made it worse: correct relative
imports widen the import evidence, so more bare calls found a "unique"
candidate. On one fixed corpus, inbound CALLS to `registry.py::ConnectionPool.get`
went 27 (staging) -> 124 (this branch head); the true number is 15.
The parser now records what the call-site file itself says the receiver is,
scoped the way Python is scoped, and the resolver honours it:
module an import binds the name to a module that resolves to an indexed
file, so the method must be a top-level name in that file.
`agent_baseline.run(...)` is `agent_baseline.py::run`, nothing else.
class an annotation, a typed parameter, an assignment from a constructor
call whose callee is a class this file defines or imports, or the
receiver naming an imported symbol. The method must belong to that
class.
builtin the name is assigned a container or scalar literal, or a
comprehension. No repository node can own the method.
unknown everything else: two contradictory bindings, a factory's return
value, or a receiver expression with no name at all
(`os.environ.get(...)`).
`builtin` and `unknown` attribute nothing — the target stays bare and the
existing unresolved path explains it. No method-name list is consulted
anywhere; every rule reads syntax in the file. Scope matters: `store` is a
`GraphStore` in fifteen test methods and a `MagicMock` in three, and only the
enclosing function says which.
Measured on one `git archive` of origin/staging built three times:
metric staging head this pass
inbound CALLS to ConnectionPool.get 27 124 15 (true: 15)
distinct callers of ConnectionPool.get 15 45 5
agent_baseline.py::run 44 0 3
agent_baseline.py::aggregate 8 0 4
path-shaped CALLS targets with no node 155 84 84
of those into __init__.py 73 0 0
resolved CALLS endpoints 10633 11781 10407
relative imports correct 0/222 222/222 222/222
relative imports correct (flask) 0/167 167/167 167/167
The previous pass claimed every withdrawn edge was a misattribution. It was
not: of the 59 edges withdrawn from those four functions, 9 were genuine.
Seven come back here (module-qualified `agent_baseline.run/aggregate`); the
two `GraphUpdateHandler` ones do not, because `handler` comes from an
unannotated factory and nothing in the file says what it holds. An honest
unresolved beats a guess.
`audit_python_imports.py` gains an attribution-precision metric, because the
headline "edges with no matching node" cannot see an edge that matches the
WRONG node — which is the failure a reviewer actually meets. Each attributed
Python call is re-read with `ast` and scored against the source at that line:
corpus staging 9235/10252 (90.1%) head 10264/11400 (90.0%)
this pass 10022/10026 (100.0%)
flask staging 305/739 (41.3%) head 359/833 (43.1%)
this pass 337/415 (81.2%)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The receiver rule landed in `_resolve_bare_endpoints` only, so the graph got more honest and the read path did not. `callers_of` falls back to `iter_edges_by_target_name(node.name)`, which answers from every CALLS edge whose target is the bare method name -- exactly the attribution the resolver had just refused to make. Converting a wrong qualified target into an honest bare one therefore handed it straight back through the tool. Measured through `query_graph` on one fixed corpus built three times (staging -> receiver commit -> here): ConnectionPool.get 421 -> 421 -> 5 (5 true, 5/15 sampled before) GraphStore.close 343 -> 365 -> 189 (189 true; 176 false dropped) GraphUpdateHandler.start 14 -> 29 -> 0 (2 of 29 were true) GraphUpdateHandler.stop 3 -> 10 -> 0 (2 of 10 were true) _sanitize_name (control) 28 -> 28 -> 28 (unchanged, all true) `GraphStore.close` was worse through the tool than on staging: the resolver withdrew 113 wrong qualified attributions and the fallback re-absorbed them. `GraphStore.receiver_evidence_admits` runs the single candidate being asked about through the resolver's own `_receiver_backed_candidates`, so there is one rule, in one place. An edge with no receiver at all -- a plain `foo(...)` -- is admitted unchanged, since that is the case the fallback exists for. Applied to: * `callers_of` -- the CALLS fallback. This is the measured one. * `tests_for` -- `get_transitive_tests` has its own bare-SOURCE fallback, and TESTED_BY is minted from the test's own CALLS edge with the metadata copied, so 5347 bare-source TESTED_BY edges here carry receiver evidence. * `references_to` -- shares the same helper. Consistency only: the parser stamps receiver evidence on CALLS (inherited by TESTED_BY) and on nothing else, so no REFERENCES edge in this graph carries any. Not applied to `get_impact_radius`: both engines traverse exact qualified-name joins, so a bare target matches no node and the fallback does not exist there. Cost, stated plainly: the two real callers of `GraphUpdateHandler.start` and `.stop` are lost, because `handler = _create_watch_handler(...)` is an unannotated factory and nothing in the file says what it holds. That is the same trade the resolver already makes, now made consistently. tests/test_receiver_evidence_read_path.py: 12 cases at the tool boundary, 10 of which fail on the previous commit for the diagnosed reason. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The receiver-evidence filter is only as good as the reader feeding it. A binding form the reader cannot spell leaves the receiver unknown, and an unknown receiver attributes nothing. That is right when the file says nothing and a silent recall hole when the file says it plainly. Four shapes were skipped, each a common way to write Python: - `store, root = _get_store()` where the function is annotated `-> tuple[GraphStore, Path]`. The target is a pattern rather than a name, so the whole binding was dropped. Now each position is bound from the annotation, including nested targets and `with ... as (a, b)`; a starred element gives up only on itself. - `code_review_graph.graph.GraphStore(...)`, a constructor reached through a dotted module path, recognised before only as a bare name. - `CodeParser().parse_file(...)`, a call on a constructor expression: no receiver name exists, but the class is written right there. - `s = _local_factory()` where a module-local def carries a return annotation. The annotation is the file speaking. No return type is read out of a function body, so nothing here is inference. Measured on one fixed corpus built by each version, staging / before / after: callers_of(GraphStore.close) 320 / 182 / 251 tests_for(GraphStore), direct 419 / 305 / 358 callers_of(_sanitize_name) 28 / 28 / 28 control, unchanged callers_of(ConnectionPool.get) 409 / 5 / 5 precision held Of 60 callers still dropped against staging, 55 are plainly wrong there: `subprocess.run` counted as a benchmark's `run`, `some_dict.get` as `ConnectionPool.get`, `tmp_path.resolve` as `CodeParser.resolve`. Two were real. The relative-import audit is unchanged at 222/222, and attribution precision holds at 10487/10491. scripts/audit_python_imports.py grows the attribution audit used for those numbers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fix(python): resolve relative imports to the module file, not the imported symbol
…e-review-graph into HEAD # Conflicts: # pyproject.toml
…e-review-graph into HEAD # Conflicts: # pyproject.toml
test(e2e): build this repository through the public pipeline
# Conflicts: # pyproject.toml
test(visualization): real-browser tests for the generated graph page
tirth8205
added a commit
to gowrishacv/code-review-graph
that referenced
this pull request
Sep 18, 2026
Three reviews of the previous commit found the automation trusted three things it had no right to trust, and asserted a fourth that is not true. **It merged whatever `gh pr list --head staging` returned.** That filter matches a branch called `staging` in any repository -- `gh pr list --help` says outright that `<owner>:<branch>` syntax is not supported -- and this repository is public with 2,871 forks. An outsider opening `theirfork:staging -> testing` became the PR the daily job resumed, had its body rewritten, and was merged: `testing`'s ruleset requires zero approvals and their own `pull_request` run satisfies the eight contexts. The same defect had a no-effort denial-of-service form, a merely conflicting fork PR making every run decide BLOCKED and exit green for ever. **It merged a PR number, and a PR's base branch is mutable by its author.** Repointing the base at `main` starts no `pull_request` run, so the green checks stay put, and `main`'s ruleset is identical to `testing`'s. Every "this can never target main" control -- the env vars, `assert_safe_targets`, the YAML test -- constrained what the job asks for, none constrained what the PR it merges is. Selection now requires the PR to be same-repository, aimed exactly `staging -> testing`, and to carry a new `auto-promotion` label, and `verify` re-reads all of it from the API seconds before the merge. The label also fixes a third case: a promotion PR the maintainer opened by hand and deliberately did not merge used to be indistinguishable, so it was silently merged with its body overwritten first. It is now refused by number, with the reason. **The commit whose checks were read was not the commit that got merged.** The PR head tracks a branch, so a push mid-run moved it. `headRefOid` was fetched and never read. It is compared now, and `gh pr merge` passes `--match-head-commit`, which GitHub enforces. **The premise that a GITHUB_TOKEN PR triggers no `pull_request` run is false.** PR tirth8205#991, opened by app/github-actions, produced a ci.yml run three seconds later; PR tirth8205#1040 re-queued all eight already-green contexts on the same SHA and sat BLOCKED for fourteen minutes. The merge step waited sixty seconds and polled `mergeable`, which is only the conflict computation and says MERGEABLE throughout. It now polls `mergeStateStatus` for up to thirty minutes. Every refusal also used to blame the ruleset's require_extra_approval_for_unattributed_changes; that rule has never fired here -- tirth8205#991 merged under it with zero reviews -- so the refusal quotes GitHub and lists causes in the order they actually occur. Also: the PR number is read from the URL `gh pr create` prints, because re-listing raced GitHub's index and an empty answer skipped the merge, the refusal report and everything else while the run stayed green and the summary said "Promoting"; a failed create names the repository setting that causes it, *Allow GitHub Actions to create and approve pull requests*, which is off today and has already failed the manual Promote workflow; a run stuck on its own PR is red rather than a green warning every morning; a `testing` tip the release gate never ran on is repaired by the next run; a failed gate on `testing` blocks tomorrow's promotion, which is what replaces the maintainer's per-landing decision; and ci.yml is no longer dispatched -- those contexts are satisfied by the manual `testing -> main` PR's own run, and dispatching it switched on a manual-only 45-minute job that is not a required context. uv run pytest tests/ -q: 4171 passed, 842 skipped. ruff and mypy clean. actionlint clean on the workflow. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
13 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Promote
staging->testingMerge with a merge commit (not squash) so every author stays on their commits.
Included
INHERITSedges #834) (Tirth Kanani)INHERITSedges #834, fix(review): use merge-base for branch diffs #847, Address August sweep follow-ups #874, Import PHP include and require targets #875, watch ignore snapshot, partial-overlap root guard, and Qoder skills missing from wheel installs #909, watch: a failed promotion releases every watch first, so the supervisor can end up watching nothing with no retry #927, fix: read source files as UTF-8 regardless of system locale #928, A persistently failing file makes every incremental update a full rebuild (identity-version loop) #944, [Bug]: installed pre-commit hook omits --repo, so committing from a git worktree builds a second full graph there #953) (Tirth Kanani)INHERITSedges #834) (Tirth Kanani)Pull requests referenced
INHERITSedges #834