fix(tools): enforce reserved namespace boundary - #424
Merged
Conversation
andrei-hasna
added a commit
to hasna/repos
that referenced
this pull request
Jul 26, 2026
De-duplication was systematically selecting a worktree. The final rank tiebreak is `id DESC`, and worktrees are indexed after the clone they came from, so among copies tied on remote-match and timestamp — which is every copy, because one fan-out sync writes them all together — the newest-inserted row won, and that is a worktree. On the live index 38 of 50 pr-queue items carried a derived path; `hasna/codewith#424` resolved to a `wt_20260711113204` directory belonging to an unrelated task. The seed body's `Repository:` line routes an agent there, so this pointed work at other tasks' worktrees. This was a regression introduced by de-duplication: the producer used to emit every copy, so the primary clone was at least present among them. Now exactly one row survives, which makes choosing the right one load bearing. A primary-clone-over-derived-copy term is added to the ranking, below freshness so a stale primary still cannot beat a copy that actually saw the merge. Measured on the live index: 38 of 50 derived, now 3 of 50, and all three are remotes with zero primary clones indexed — the correct fallback. The path markers move into shared constants so the TypeScript predicate and the SQL ordering cannot drift into two definitions. The queue also named the wrong repository. Filtering moved to the PR-derived owner but `githubFullNameFromRepoRow` still read the winning repo record, so a pull request recorded against an unrelated checkout was queued under that checkout's identity: `hasna/aicopilot#9` produced the fingerprint `github-pr:hasna/aicopilot#9`. The fingerprint is the task identity key, so this collides and mis-routes tasks. The pull request's own org/repo now win. Allowing partial GraphQL responses through opened a crash path: a partially-resolved page arrives as nodes containing nulls, and those were pushed unfiltered, so `pr.number` threw and aborted the sync. Node collection is now a filtered, separately tested function. Sync accounting used three units in one object. `synced` counted rows written across checkouts while being reported as pull requests synced — 23x inflated for codewith, 115x for platform-alumia — and the --sync-max-repos budget subtracted a remote count from a repo-denominated allowance. `synced` is now distinct pull requests, `rows_written` carries the per-checkout total, and every counter in the pr-queue `synced` object is documented with its unit. Also: - A null pull request connection is no longer reported with a message that MISSING_REPO_PATTERNS matches, so a transient failure surfaces as an error instead of being filed as a renamed or deleted repository. - PrRow's repo columns are typed nullable, matching the outer join they come from, and an orphaned row no longer renders `Repository: null`. - The still-open lookup in applyPullRequestTerminalStates is chunked so it cannot exceed SQLite's parameter limit. - Documented that `state` selects how much history to page, not which rows are written: the open set is always fetched because reconciliation is judged against it.
andrei-hasna
added a commit
to hasna/repos
that referenced
this pull request
Jul 26, 2026
) * fix(prs): reconcile terminal states and de-duplicate the PR surface `repos prs` could not be used as a source of truth for pull requests. Terminal state was never reconciled. `sync-github` only inserted and updated what it fetched, so a PR merged or closed upstream stayed `open` in the index forever and `--state open` grew without bound. On station01 a full successful sync (4703 PRs across 142 repos, index updated_at moved to 2026-07-26T11:02:04Z) still left `--state open` reporting 440 rows for the hasna org against 10 live on GitHub. Syncing more often cannot fix this; the reconciliation pass had to be written. After fetching the complete open set, rows still marked open but absent from it are re-queried and driven to their real terminal state. Repos whose open set is now empty are still visited, so their stale rows drain too. Rows were also duplicated, but not for the reason the symptom suggested: UNIQUE(repo_id, number) is present and has zero violations. The same GitHub repository is indexed once per local checkout — worktrees, throwaway build copies, `open-*` mirrors — and every one of those rows carries its own copy of the same PR. github.com/hasna/codewith maps to 23 repo_ids, so its PRs were reported 23 times; 829 surplus open rows, not 39. `repo_id` is therefore not the identity of a pull request and the URL is, so the listing de-duplicates on it, preferring the copy whose repo record actually owns the PR and a terminal state over a stale `open`. State is filtered after de-duplication, not before, so a reconciled `merged` row can supersede a stale `open` one. `--duplicates` restores the per-checkout view. Sync now fans out by remote instead of by directory: one fetch updates every checkout of that repository, so unvisited checkouts no longer keep stale rows alive. Adds the fields a merge gate needs — head_sha, mergeable, merge_state_status, ci_state, is_draft, review_decision — from the GraphQL repository.pullRequests connection. `gh search prs` is not used: its index is stale and on 2026-07-26 reported five closed/merged hasnaxyz/iapp-factory PRs as open while missing the one genuinely open PR. mergeStateStatus needs a preview media type, so it is requested with that header and silently dropped if refused. Adds `--org`/`--repo-name`, resolved from each PR's own URL rather than the owning repo record — a PR mis-attributed to an unrelated record (hasna/aicopilot#9 stored against platform-aicopilot) is now attributed to the org that actually owns it. Rows carry `org` and `repo` names. Also: - `repos --json` and `prs --json` announce truncation on stderr with the true total instead of silently stopping at the page size. - `repo`/`show`/`inspect`/`cd`/`open` accept `--remote host/org/name` for exact, deterministic targeting, and `cd`/`open` accept `--exact`. The fuzzy default resolves `todos` to whichever of open-todos and platform-todos it reaches first, which automation cannot rely on. - A scan that cannot READ a remote no longer erases the indexed identity. It previously nulled remote_url whenever `git remote get-url origin` failed, which is how 757 of 1243 records lost their remote. Supplying a remote that fails sanitization still clears it, preserving that invariant. - `git -C` searches upwards, so a directory with a gutted .git answered with its ANCESTOR's remote and would have stamped a parent project's identity onto an unrelated child. The read is now rejected unless git considers that exact path the top of the working tree. - Migration 12 backfills gh_owner/gh_repo, restores remote_url from the remotes table, and adds the FTS update trigger pull_requests never had. It is written defensively so it survives divergent schemas. * fix(prs): address adversarial review of the PR surface Blockers Reopened pull requests were reported closed. The de-duplication ranking preferred a terminal state over freshness, but a terminal state is not permanent — GitHub PRs can be reopened. A copy that saw the closure beat a copy that saw the reopen, so the PR vanished from `--state open`, the tool's primary use. `updated_at` now outranks state, with the merge, close and create timestamps standing in when it is absent so a terminal row is not ranked as though it had no history. Terminal-over-open survives only as a tie-break between copies sharing a timestamp. The `open-repos.pr-queue.v1` producer — the surface automation actually consumes — was never de-duplicated. It re-joined pull_requests to repos by hand, so a repository checked out N times produced N copies of every PR: measured on the live index, a 50-item queue was 23 copies each of two pull requests, with `limit` spent entirely on duplicates. It now shares the one de-duplicated listing, so the fix reaches every PR surface rather than the CLI alone. Its `org` filter also moves off the repo record's org onto the org that owns the pull request, and an unresolvable `--repo` matches nothing instead of everything. Reconciliation issued one GraphQL query per checkout rather than per repository. Every checkout of a remote has the same stale set, so the identical question was re-asked once per local directory — `github.com/hasnastudio/platform-alumia` is checked out 115 times on this machine. The stale sets are now unioned and resolved in a single pass. The test that should have caught this asserted `fetchCalls`, which the stub only incremented for a different method; it now asserts the reconciliation queries themselves. Also - `state` was accepted and silently dropped, so `state: "open"` still paged merged/closed history. It is honoured, and the MCP schema no longer advertises behaviour that did not exist. - A partially-resolvable GraphQL response was discarded wholesale, so one bad alias killed the other 49 in its batch and aborted every remaining checkout. Partial data is now used, and a failed batch is skipped instead of abandoning the repository. - `gh api -F` applies magic type conversion: an all-numeric repository name would be sent as an Int against a String! variable, and a leading `@` reads a file. Switched to `-f`. - `--org` filtered on gh_owner but printed a repo-record fallback, so the tool could print an org that returned nothing when filtered on. The stored columns are now authoritative at write time and are what both the filter and the output use. - The `--remote` rejection path echoed the caller's raw argument, which is exactly where an embedded credential would still be present. Only the sanitized identity is printed, per the invariant stated in lib/remote-identity.ts. - Migration 12 now repairs the FTS damage it documented rather than only halting it: `INSERT OR REPLACE` plus recursive_triggers OFF had orphaned ~7 stale index entries per live row (13868 orphans for one term on the live index, now 0). The update trigger is created after the backfill so the backfill does not push two FTS writes per row. - Escaped `_` in the owner-match LIKE pattern; it is a wildcard and legal in repository names. - `applyPullRequestTerminalStates` reads which rows are still open in one statement instead of one per number, and its comment about the driver's change count is corrected. - Exported the new query and sync surfaces from the package entry point. - Tests: the fan-out test no longer spawns real `gh` subprocesses, and scanner scratch directories are ignored. * fix(prs): select the primary clone and name the owning repository De-duplication was systematically selecting a worktree. The final rank tiebreak is `id DESC`, and worktrees are indexed after the clone they came from, so among copies tied on remote-match and timestamp — which is every copy, because one fan-out sync writes them all together — the newest-inserted row won, and that is a worktree. On the live index 38 of 50 pr-queue items carried a derived path; `hasna/codewith#424` resolved to a `wt_20260711113204` directory belonging to an unrelated task. The seed body's `Repository:` line routes an agent there, so this pointed work at other tasks' worktrees. This was a regression introduced by de-duplication: the producer used to emit every copy, so the primary clone was at least present among them. Now exactly one row survives, which makes choosing the right one load bearing. A primary-clone-over-derived-copy term is added to the ranking, below freshness so a stale primary still cannot beat a copy that actually saw the merge. Measured on the live index: 38 of 50 derived, now 3 of 50, and all three are remotes with zero primary clones indexed — the correct fallback. The path markers move into shared constants so the TypeScript predicate and the SQL ordering cannot drift into two definitions. The queue also named the wrong repository. Filtering moved to the PR-derived owner but `githubFullNameFromRepoRow` still read the winning repo record, so a pull request recorded against an unrelated checkout was queued under that checkout's identity: `hasna/aicopilot#9` produced the fingerprint `github-pr:hasna/aicopilot#9`. The fingerprint is the task identity key, so this collides and mis-routes tasks. The pull request's own org/repo now win. Allowing partial GraphQL responses through opened a crash path: a partially-resolved page arrives as nodes containing nulls, and those were pushed unfiltered, so `pr.number` threw and aborted the sync. Node collection is now a filtered, separately tested function. Sync accounting used three units in one object. `synced` counted rows written across checkouts while being reported as pull requests synced — 23x inflated for codewith, 115x for platform-alumia — and the --sync-max-repos budget subtracted a remote count from a repo-denominated allowance. `synced` is now distinct pull requests, `rows_written` carries the per-checkout total, and every counter in the pr-queue `synced` object is documented with its unit. Also: - A null pull request connection is no longer reported with a message that MISSING_REPO_PATTERNS matches, so a transient failure surfaces as an error instead of being filed as a renamed or deleted repository. - PrRow's repo columns are typed nullable, matching the outer join they come from, and an orphaned row no longer renders `Repository: null`. - The still-open lookup in applyPullRequestTerminalStates is chunked so it cannot exceed SQLite's parameter limit. - Documented that `state` selects how much history to page, not which rows are written: the open set is always fetched because reconciliation is judged against it. * fix(prs): rank state above path and keep the queue payload coherent The primary-clone rank term sat one line too high, above the terminal state test. With copies tied on timestamp, a primary saying `open` beat a worktree saying `merged`, while the mirror case — primary merged, worktree open — still returned `merged`. An identical state disagreement resolved differently depending purely on which copy happened to be a worktree, which is worse than a wrong answer because it looks non-deterministic. It is reachable rather than theoretical: reconciliation writes `updated_at = COALESCE(?, updated_at)`, so a row reconciled from a response whose `updatedAt` was null flips state while keeping its old timestamp, leaving copies tied on the ranking key and disagreeing on state. The term moves below the state test, where it still decides the path in the normal case — copies written by one fan-out sync agree on state, so the state test is a tie and the path preference is what breaks it. The previous test could not catch this because it used differing timestamps; the new one uses equal timestamps and asserts both mirror images agree. The final tiebreak becomes `id ASC`. Among otherwise indistinguishable copies the earliest-indexed row is the original clone, and recency buys nothing at that point. `hasna/codewith` has two non-derived clones, and `id DESC` picked the newer side clone: codewith#424 now routes to open-codewith rather than open-codewith-infinity-attestation. `repo.name` in the open-repos.pr-queue.v1 payload had been redefined in place from the local directory name to the GitHub repository name. That left `name` and `path` describing different things — `{name: "emails", path: ".../open-mailery"}` — so the object was internally incoherent and any consumer using `name` to locate a checkout would break. It was presented as a null-safety fallback but was a semantics change on a shipped contract. `name`, `org` and `path` go back to describing the local checkout; the GitHub identity stays on `full_name` and is also exposed as new `github_repo`/`github_org` fields rather than displacing existing ones. Verified on the live index: `name` equals the directory basename for all 50 items. The SQL term and the TypeScript predicate still disagreed on case, since SQLite LIKE is ASCII case-insensitive and JS RegExp is not — a `/WorkTrees/` path was derived to one and primary to the other. Both now compare case-insensitively, and markers are asserted at load to be ASCII and free of LIKE metacharacters, so adding something like `node_modules` fails loudly instead of turning its `_` into a wildcard. Also: - collectPullRequestNodes validates every field the writer stores into a NOT NULL column, so a half-resolved node is dropped at the boundary instead of aborting a write transaction further in. - The pr-queue `synced` object is projected field by field in the all-orgs branch too; it previously passed through a superset, giving this v1 payload a key set that depended on which branch produced it. - The seed body's `Repository:` line is guarded, so an orphaned row can no longer render the string "null" where a directory belongs.
andrei-hasna
force-pushed
the
fix/runtime-drift-namespace-guard
branch
from
July 27, 2026 06:19
5e8c003 to
04c1f2c
Compare
andrei-hasna
force-pushed
the
fix/runtime-drift-namespace-guard
branch
from
July 27, 2026 06:31
04c1f2c to
86eb8eb
Compare
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Summary
tool_searchdeclarations while preserving valid tools and hosted fallbacksweb.runreserved-name exceptionVerification
59785a218reproducedimage_gen.imagegenserializationc8b41c677d06903d7d1b25c71c698dde510f678cjust fmtandgit diff --checkgitleaks protect --staged --redact --no-banner: no leaksgitleaks detect --source . --log-opts=origin/main..HEAD --redact --no-banner: no leaksweb.*, and nested dispatch were reconciled inbbcead840Scope
This is the separate residual defect tracked by OPE2-00147/OPE80-00050. It does not attribute the stale 0.1.67 runtime incident to this guard. No runtime, installer, release, or publishing changes are included.
Refs #255. The issue remains open for the broader acceptance matrix.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.