Consolidate the SWE and PR-AF nodes in the app, and make installs superseded-aware - #873
Merged
Merged
Conversation
…lly installed `Manager.run` inferred the installed package by diffing the registry's names before and after: whichever name is new must be the one this job installed. That inference breaks on exactly the case `superseded_by` was added for. A successor may declare its predecessor's name — an in-place rename, which is what both Agent-Field/SWE-AF#122 and Agent-Field/pr-af#64 use, and what keeps a node id, its triggers, and its node-scoped secrets intact across the swap. The set of installed names is then identical before and after, so the diff finds nothing and the job reports an empty package name. AgentField Desktop streams that job's output, so the user watched a successful install end in "install completed: " with the name missing. When the successor's name *does* differ, the diff happened to work, but only by luck: it returns the first registry name that is new, so any unrelated entry appearing during the install is misattributed to this job. The installer already knows the answer — `GitInstaller` tracks it in `installedName` and propagates it through a redirect. Export it, thread it out through the package service as `InstallPackageWithResult`, and have the job prefer it, keeping the before/after diff as the fallback for installers that cannot report a name. Node-dependency discovery used the same diff idiom and is switched to the authoritative name too, which also stops it from walking the dependencies of a package some other caller installed concurrently. Updates take the authoritative name as well. `StartUpdate` pre-seeds the job with the name being updated, so previously the installer's answer was discarded — and an update whose recorded source redirects to a differently named successor would then try to restart the package the redirect had just uninstalled. It now reports and restarts the node that exists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every install result in the app was phrased from the request: the catalog row's
name, or the URL that was pasted. A `superseded_by:` redirect makes that a
guess — the manifest at the source hands the install off to a successor, which
may register under its own name.
Now that the control plane reports what it installed, repeat that instead:
- a pasted repo says "pr-af installed" rather than "Installed from
https://github.com/Agent-Field/pr-af", which is the more useful half of the
sentence and the only one that tells you what to run next;
- a catalog install names the successor if it ever disagrees with the row —
the two agree for every entry today (that is the invariant catalog.ts
documents), so a disagreement is drift worth seeing rather than hiding
behind the row's own label;
- an update that followed a rename reads "<old> replaced by <new>" instead of
claiming it updated a node that no longer exists.
Each falls back to the previous wording when the control plane names nothing,
so an older control plane behaves exactly as it does today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… by repo The catalog offered PR-AF twice — a Python row and a Go row shipping the same reasoners under a name the user had to know to type. The two were indistinguishable in the Install view except by the `-go` suffix, which is an implementation detail leaking into a product list. Agent-Field/pr-af#64 collapses them the way Agent-Field/SWE-AF#122 collapsed the SWE fleet: the root manifest redirects to `//go`, and the Go node takes the product's name. So this is one `pr-af` row, language go. Both consolidated rows now install from the bare repo URL rather than naming `//go` directly. Selecting the subdirectory would install the same node, but it skips the redirect — and the redirect is the part that carries an existing install across: it puts the successor in place first, migrates node-scoped secrets, and only then retires the predecessor. Someone who already has the Python node gets migrated by pressing Update; someone naming `//go` would only collide with it. Naming the repo and letting the manifest decide is also simply what a user can be told to type. That changes the rule both catalogs are written against, so both header comments now say the new one: an entry's `name` must equal the name the package ends up REGISTERED under once the install settles, which under a redirect is not the `name:` in the manifest at the source, and may live in a subdirectory the catalog never mentions. sec-af and cloudsecurity-af are untouched — neither ships a second implementation, so neither has anything to collapse. The SWE guard test generalizes to cover both repos: exactly one row per repo, named for the product, sourced at the bare URL, language go, and the retired implementation-suffixed name absent from the whole catalog — so a re-added row fails here instead of quietly reappearing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The agentfield-use skill is what a harness reads to learn how to call the nodes on this machine, and skillkit installs it into Claude Code, Codex, Cursor and the rest — so its examples are the ids an agent will actually try. Its `executions/active` sample still showed a run targeting `pr-af-go`, a name that stops existing once Agent-Field/pr-af#64 lands. Applied identically to the embedded copy under skillkit/skill_data so the two stay byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…installed The job reaches the authoritative name through a type assertion, and a failed assertion is silent — it falls back to inferring the name from a registry diff, which is exactly the path that returns nothing for an in-place `superseded_by` replacement. Every other test in this file uses a stub that satisfies the interface by construction, so none of them would notice a production wiring change (a decorator, a swapped implementation) that quietly reverted the fix. This one asserts against the service the server actually constructs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
📊 Coverage gateThresholds from
✅ Gate passedNo surface regressed past the allowed threshold and the aggregate stayed above the floor. |
Contributor
📐 Patch coverage gateThreshold: 80% on lines this PR touches vs
✅ Patch gate passedEvery surface whose lines were touched by this PR has patch coverage at or above the threshold. |
Switching dependency discovery from a registry snapshot to the authoritative installed name dropped the only thing that stopped a cycle. The snapshot version terminated by accident but reliably: the recursive call received a snapshot that already contained the package just reinstalled, so the second lap skipped it. Recursing on a single name removed that, and the remaining guard — `depName != "" && isPackageInstalled(depName)` — cannot substitute. It only knows a dependency's name for `af://registry/…` refs, and a forced install reinstalls whatever is already there. Every update is forced (`StartUpdate` → `startJob(JobUpdate, …, true)`), so two packages declaring each other by bare git URL or local path recursed until the process died — with the package-job manager's `active` latch held, blocking every later install. Tracks the packages this install pass has walked instead, which does not depend on ref form, on Force, or on registry state. The accompanying suite pins the seam's behaviour end to end through the real git installer rather than a stub: a redirect reports the successor — including when the successor takes the predecessor's own name, the case a registry diff cannot see and the reason this seam exists — a failed install reports no name at each stage it can fail, an uninstallable dependency does not fail its parent, and a cycle terminates. That last one fails in 30s against this fix reverted. `manager_test.go` covers the other side: an installer that cannot report a name still installs and falls back to the registry diff, so the old path stays intact for anything that does not implement the newer seam. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 4, 2026
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
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.
Agent-Field/SWE-AF#122 and Agent-Field/pr-af#64 each collapse a repo's Python and Go nodes into one, using the
superseded_by:manifest key from #864. This is the AgentField side of that: one catalog row per product, and an install path that is honest about what a redirect actually installed.One PR-AF row, and both consolidated nodes install by repo URL
The catalog offered PR-AF twice — a Python row and a Go row shipping the same reasoners under a name the user had to know to type. In the Install view the two cards were indistinguishable except for the
-gosuffix, which is an implementation detail leaking into a product list. Now onepr-afrow, language go, mirroring what #864 already did for SWE.Both consolidated rows install from the bare repo URL rather than naming
//go. Naming the subdirectory installs the same node, but it skips the redirect — and the redirect is the part that carries an existing install across: it puts the successor in place first, migrates node-scoped secrets, and only then retires the predecessor. Someone already on the Python node gets migrated by pressing Update; someone naming//gowould only collide with it.That changes the rule both catalogs are written against, and both header comments now say the new one: an entry's
namemust equal the name the package ends up registered under once the install settles. Under a redirect that is not thename:in the manifest at the source, and it may live in a subdirectory the catalog never mentions.sec-af and cloudsecurity-af are untouched — neither ships a second implementation, so neither has anything to collapse.
The install job was reporting the wrong package — sometimes nothing at all
Manager.runinferred the installed package by diffing the registry's names before and after the install. That inference breaks on exactly the casesuperseded_byexists for.A successor may declare its predecessor's name — an in-place rename, which is what both repo PRs use and what keeps a node id, its triggers, and its node-scoped secrets intact across the swap. The set of installed names is then identical before and after, the diff finds nothing, and the job reports an empty package name. The desktop app streams that job's output, so the user watched a successful install end in
install completed:with the name missing.When the successor's name does differ the diff happened to work, but only by luck — it returns the first registry name that is new, so an unrelated entry appearing during the install is misattributed to this job.
The installer already knows:
GitInstallertracksinstalledNameand propagates it through a redirect. This exports it, threads it out through the package service asInstallPackageWithResult, and has the job prefer it — keeping the before/after diff as a fallback for installers that cannot report a name. Node-dependency discovery used the same idiom and is switched over too.Updates take the authoritative name as well.
StartUpdatepre-seeds the job with the name being updated, so the installer's answer was previously discarded — and an update whose recorded source redirects to a differently named successor would then try to restart the package the redirect had just uninstalled. It now reports and restarts the node that exists.The app says what landed
Every install result was phrased from the request — the row's name, or the URL pasted. With the real name available, the app repeats that instead: a pasted repo says
pr-af installedrather thanInstalled from https://github.com/Agent-Field/pr-af; a catalog install names the successor if it ever disagrees with the row (drift worth seeing, not hiding); an update that followed a rename reads<old> replaced by <new>. Each falls back to the previous wording when the control plane names nothing, so an older control plane behaves exactly as today.The
agentfield-useskill'sexecutions/activeexample still targetedpr-af-go. skillkit installs that skill into Claude Code, Codex, Cursor and the rest, so its examples are the ids a harness will actually try — updated in both the source and embedded copies, which stay byte-identical.Validation
Ran the literal steps from
.github/workflows/desktop.ymlandcontrol-plane.yml:npm ci,npm run typecheck,npm testindesktop/— 393 passed (19 files)go build ./...,go vet ./...incontrol-plane/— cleango test ./...— every package passes exceptTestDevServiceRunDev, which fails identically on unmodifiedorigin/main(same 3 subtests, same 120s port-discovery timeouts; verified in a detached worktree at 4bc8ce7). Pre-existing, environment-specific, untouched by this diff.skillkit,cli,packages,packagejobs— all green, including the skill mirror byte-identity test.New tests are derived from behaviours, not from the code: a same-name redirect reports that name rather than empty; a different-name redirect reports the successor; a plain install is unchanged; a failed install claims no completion; an unrelated concurrent registry addition is not misattributed; an in-place replacement leaves exactly one registry entry recorded against the successor's source; an update that renamed restarts the successor. The catalog guard test generalizes to both repos — one row each, product-named, bare URL, language go, and the retired
-goname absent from the whole catalog, so a re-added row fails here instead of quietly reappearing.Merge order — this lands FIRST
Merge and release this before Agent-Field/pr-af#64 and Agent-Field/SWE-AF#122. That is the opposite of what it looks like, so the reasoning:
Selecting a subdirectory bypasses the redirect.
resolvePackageRootresolves//goandparsePackageMetadatathen reads onlygo/agentfield-package.yaml— the root manifest, and itssuperseded_by, is never consulted (git.go,InstallFromGitsteps 2–4). So the moment a repo PR renames itsgo/manifest, every already-shipped client carrying a<node>-gocatalog row pointed at//gostarts installing a package under a different name than the row claims.Verified against a real installer built from
origin/main, using a local repo fixture carrying pr-af#64's manifests:pr-af-goinstalls a package registered aspr-af, with no redirect warning — so its card never flips to Installed, while the other row's card does;pr-afgets a hardpackage pr-af already installed (use --force to reinstall)— a click that succeeds today;af run pr-af-go, which a shippedaf catalogstill advertises, fails:agent node pr-af-go not installed.v0.1.120ships that row (git show v0.1.120:desktop/src/shared/catalog.ts), and no merge order protects binaries already on disk — they read the manifest live. But landing this first removes the stale rows from every subsequent build, so the exposed population is only users who have not updated yet.It also fixes a desync that exists today: main's
swe-plannerrow is sourced atSWE-AF//go, whose manifest is stillswe-planner-go, so that row currently installs under a name the catalog does not match. Repointing it at the bare repo URL makes the name agree again in the pre-#122 window (it installs the Python node until #122 lands — thelanguage: golabel is the only thing that reads early).🤖 Generated with Claude Code