Skip to content

Use HTTP for Collie's forge operations with the CLI's login - #194

Merged
madskronborg merged 15 commits into
masterfrom
madskronborg/forges-transport
Oct 23, 2026
Merged

madskronborg merged 15 commits into
masterfrom
madskronborg/forges-transport

Conversation

@madskronborg

@madskronborg madskronborg commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Move Collie's GitHub and GitLab reads and writes to cached in-process forges providers using the existing gh/glab login. This removes repeated CLI startup from CI, merge-request and release polling while preserving forge behavior, credential refresh and safe write handling. CLI commands remain for agent instructions, credentials and repository setup. The fake forge now serves HTTP to spawned hosts as well as in-process callers.

Verified on 336f9503 by GitHub CI: all four jobs passed. The full suite passed 4,364 tests with zero failures and 11 existing optional live-harness skips; build, smoke and the pinned herdr contract check also passed. Focused local checks through Collie passed 284 baseline cases and 244 forge/release/verification cases under CI's Bun 1.4.4 and four workers, plus typecheck, lint and formatting.

The earlier Collie check gate passed on 92e08921 before the CI repairs and master integration; that historical gate is not offered as proof of the new tree. Current-head CI verification is recorded as ci-integrated-github.

Required verification:

  • check: /home/mk/.local/share/mise/installs/bun/latest/bin/bun run check (in /home/mk/.herdr/worktrees/.collie/madskronborg/forges-transport)

The changes after the last review have the test and gate evidence described above but were not re-reviewed:

  • Serve both fake forge hosts throughout every baseline scenario and follow fixture replacements, preventing fake hostnames from reaching the real network.
  • Supply fixture credentials, assert CI requests and retries over HTTP, and bound direct workflow sessions to 30 seconds.
  • Publish the command fixture's PID atomically so cleanup assertions cannot mistake an empty PID file for process group zero.
  • Gate shutdown races at child creation, independently of transient process metadata.
  • Match the toolchain fixture's complete # v2 marker so temporary paths cannot suppress the intended upgrade; retain a deterministic path regression.
  • Scope verification repositories to the tests that use them, with cleanup inside each test's promise instead of file-wide hooks.
  • Resolve the conflict with master's Stage workflow using the shared HTTP transport, preserving selected-job attribution and paginating environments, deployments and job attempts.

One non-blocking finding remains: test/desktop-peers.test.ts:65 times out when run from sources because its upgrade fixture copies Bun rather than the complete source command. This reproduces on released base b4ef74af; the compiled test and complete final gate pass.

Not settled by the run

Recorded implementation assumptions
  • assumed in 01-forge-http-and-the-ci-job-reads.md: The CLI front door (src/collie.ts) is given forgeHttpLayer too, though no CLI command runs the moved operations yet, so a later command reaching them cannot start without it.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: ForgeHttp runs credential reads through its own Runner captured at layer build; the adapter's run argument is no longer used by the four moved operations.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: forges is imported lazily inside src/forge/http.ts so a start that asks no forge never loads it (docs/internals.md keeping a start fast); smoke greps the binary for it because of that.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: GitLab's runStatus reads expiry and token for a stored login under env -u GITLAB_TOKEN -u GITLAB_ACCESS_TOKEN -u OAUTH_TOKEN, cached under its own key, so an environment token is never sent to a host from agent-supplied text.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: An OAuth expiry glab stores that Collie cannot parse is treated as non-expiring, so the token is used and a 401 then drives the one re-read.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: A glab expiry whose zone is an abbreviation (e.g. CET) is read in this Machine's local zone, as Go writes it.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: The GitLab no-login runStatus criterion already held on the base (it ran glab auth status first), so its red was shown by mutating the guard rather than on the base.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: test/mr.test.ts's recorder test that runStatus never asks an unlogged host was replaced by the forge.test.ts case, which asserts the same on both faces.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: Failure reasons keep their shape (<Display> answered <status>: …, could not be reached) but their wording now comes from the HTTP answer rather than CLI stderr.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: COLLIE_FORGE_API is the name of the base URL a test points a host at to reach the fake's HTTP face; unset in production.
  • assumed in 01-forge-http-and-the-ci-job-reads.md: The benchmark's first library read includes the cold credential read and connection, as a host's first poll would.
  • assumed in 02-the-merge-request-panel-and-a-branchs-merge-requests-over-http.md: The planner's answer in attachments/message-42165507.md is authoritative with the ticket unchanged: GitHub REST auto_merge supplies forBranch's autoMerge, and no GraphQL call is added for that branch lookup.
  • assumed in 02-the-merge-request-panel-and-a-branchs-merge-requests-over-http.md: The replacement implementer retained the previous implementer's completed commit 894200a and recorded benchmark, confirmed the recorded failing-first cases, ran the remaining checks and confirmed the remote push rather than recreating completed work.
  • assumed in 02-the-merge-request-panel-and-a-branchs-merge-requests-over-http.md: The source-only peer-upgrade failure is reported as a pre-existing minor finding because it reproduces on the released base and its repair is outside ticket 02.
  • assumed in 03-branch-protection-and-deploy-status-over-http.md: The ticket's explicit requirement to page GitLab protected_branches takes precedence over the old one-page unknown fallback; wildcard matching stays unchanged.
  • assumed in 03-branch-protection-and-deploy-status-over-http.md: GitLab liveTier keeps its existing environment and 50-deployment reads, while GitHub's deployment list is paged as explicitly required by the ticket.
  • assumed in 04-issues-posting-and-the-login-through-forges-own-operations.md: The planner amended ticket 04 to move only the six named operations and created ticket 05 for the remaining review/release operations, so readReview keeps its private CLI login read until that ticket rather than partly migrating its implementation here.
  • assumed in 04-issues-posting-and-the-login-through-forges-own-operations.md: The planner’s single-authority answer in attachments/message-9feb6a4d.md preserves post.ok:true after a note lands and approval fails, with inspection guidance before approving again, because retrying the whole post would duplicate the note.
  • assumed in 05-review-reads-replies-merge-and-release-pipelines-over-http.md: GitHub readReview reuses the existing paginated panel query extended with mergedBy, preserving all review fields and avoiding a second implementation of check-context pagination.
  • assumed in 05-review-reads-replies-merge-and-release-pipelines-over-http.md: Per the planner's pane-only instruction in message-1e8c2ee6.md, the delivering fixture regression from ticket 02 is corrected in this slice after original-base proof and bisection, by serving the same model over HTTP rather than changing the timeout or adapter behavior.
  • 01-forge-http-and-the-ci-job-reads.md: after merge: Guard: on a real Run on cego/collie with a check left to CI, the card shows the job's state as before.
  • 02-the-merge-request-panel-and-a-branchs-merge-requests-over-http.md: after merge: Guard: on a real Run on cego/collie, the card's panel shows the pull request's checks, review decision and auto-merge as before.
  • 02-the-merge-request-panel-and-a-branchs-merge-requests-over-http.md: after merge: Guard: on a branch whose pull request merges itself once green, a push step still parks with not pushed: auto-merge is on for <url>.
  • 03-branch-protection-and-deploy-status-over-http.md: after merge: Guard: a merged cego/collie pull request shows its deploy line as before.
  • 04-issues-posting-and-the-login-through-forges-own-operations.md: after merge: Guard: self-improve on cego/collie files an issue and comments on one, and the Run's record has their URLs.
  • 04-issues-posting-and-the-login-through-forges-own-operations.md: after merge: Guard: posting a clean review on a cego/collie pull request leaves one comment and an approval.
  • 05-review-reads-replies-merge-and-release-pipelines-over-http.md: after merge: Guard: a landing Run on cego/collie reads its review, replies to a comment and merges as before.

The recorded ticket-01 smoke assumption was superseded during review: the final smoke exercises the compiled CLI against the HTTP fake and checks its response and request. Ticket 05 completed the review/release operations deferred by ticket 04.

Mads Kronborg and others added 11 commits October 23, 2026 08:12
A host polls `job`, `baseJob`, `retryJob` and `runStatus` for every Run
waiting on CI, and each poll spawned several `gh`/`glab` processes. ForgeHttp
asks the same routes through the `forges` library instead, with the token
read from the CLI the human signed in, so no new credential is introduced and
the benchmark on this Machine shows the library no slower than the CLI on
either forge. Only `src/forge/` may import the library, and it is loaded
lazily so a start that asks no forge never pays for it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The panel and the branch lookups ran gh or glab on every poll and every
push; measured on this Machine, asking the forge directly is faster on
both forges (GitHub 716 -> 572 ms, GitLab 727 -> 301 ms median for the
panel). GitHub's panel is one GraphQL query for every field gh pr view
asked for; GitLab's reads the merge request, its approvals and its
discussions together. A branch's merge requests are the REST list
filtered by source branch, read to its last page.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keep agent launch decisions and deployment polling on ForgeHttp with the CLI's cached login. Preserve unknown protection on failures, page GitLab's protection rules and GitHub's deployments, and keep the caller fixtures answering on their HTTP face.
Use the CLI's cached login for issue publication, review posting and personal merge-request menus. Preserve partial review success and the host's privacy checks without resending uncertain writes.
Read review and release state with the CLI's cached login so release watches reuse HTTP connections. Read back uncertain merge writes without resending them, and bind agent-supplied GitLab pipeline URLs to their host's stored login.
Serialize credential reads by host so normal and stored-login caches cannot race GitLab's rotating refresh token. Keep the stricter token cache separate.
Guard side-effect imports of forges and its subpaths so they cannot bypass the adapter boundary.
Exercise the compiled runner's lazy provider against the existing HTTP fake so an omitted forge dependency fails the smoke gate.
Remove comments that repeat field and parameter names without explaining a constraint.
Prevent workflow tests from reaching fake hostnames on the real network after the transport cutover. Serve both forge models for each scenario, follow model replacements and bound host sessions so a missing fixture cannot stall the suite.
Publish the fixture PID atomically so a reader cannot turn an empty file into process group zero and report that an already stopped child is still running.
@madskronborg madskronborg self-assigned this Oct 23, 2026
Mads Kronborg added 4 commits October 23, 2026 15:29
Avoid missing the startup barrier when a new process has not yet published the command line and environment the old proc reader matched. Gate the real spawner directly, release it on test cleanup and assert that another Run's processes survive the stop.
Require the complete version-marker line so a temporary path containing v2 cannot suppress the fixture's tool upgrade. Keep such a path in the baseline regression so both workflow variants prove the check runs again after the upgrade.
Finish repository cleanup inside each test's Effect scope before its promise resolves. Pure snapshot tests no longer allocate a Git fixture, and filesystem tests no longer rely on separate cleanup hooks racing the suite's leak check.
@madskronborg
madskronborg merged commit d8d8be9 into master Oct 23, 2026
5 checks passed
madskronborg pushed a commit that referenced this pull request Oct 23, 2026
#194 made the host's placement need ForgeHttp, which reviewedBranch now gets from the
host's services as checkoutReviewed does, and gave worldWith an adapters object.
madskronborg added a commit that referenced this pull request Oct 26, 2026
#198 was written before #194 moved forge reads onto HTTP, so its three
scenarios still stubbed the glab CLI and their forge reads went
unanswered, leaving master red.

Co-authored-by: Mads Kronborg <mk@cego.dk>
madskronborg added a commit that referenced this pull request Oct 26, 2026
)

* Offer a review's fix only where it can run, and log a refusal once

A review offered Fix findings for a target with no branch to fix, refused
every answer the same way, and replays wrote the refusal again each time.
The review now asks the host before its first menu whether the reviewed
branch can be established, by the checkout's own resolution and cutting
nothing, and withholds what cannot run, naming why. A fix refused when it
starts stands for the rest of the Run, and each refusal is recorded inside
the Activity that met it.

Closes #185.

* fixup! Offer a review's fix only where it can run, and log a refusal once

* fixup! Offer a review's fix only where it can run, and log a refusal once

* fixup! Offer a review's fix only where it can run, and log a refusal once

* Wait for a held check's record before ending its host in tests

The host writes a check's running-checks record only after spawning it, but
the tests ended or restarted the host as soon as the child noted its pid. On
a loaded machine the record was not there yet, so the next host found the
check by its markers alone and the record count was zero. Both tests failed
on master the same way.

* Rerun the gate now desktop dependencies are installed

The gate's check failed at 064b000 only because this worktree had no
desktop/node_modules, which master's typecheck now reads. Installing them
changes nothing git tracks, and Collie does not rerun a Run's check on a
tree it already failed on.

* Write a parked reason by renaming it into place

A reader that found the file between its creation and its write read an empty reason,
which failed the idle-budget test in CI and would show the board a Run parked for nothing.

* Keep git's detached maintenance out of test repositories

Git 2.47+ (CI has 2.55) detaches the maintenance a commit starts, so it can still be
writing into a test's repository after a fast test removed it, and the suite reports the
directory as left behind.

* Fit the reviewed branch and its test to master's forge and test APIs

#194 made the host's placement need ForgeHttp, which reviewedBranch now gets from the
host's services as checkoutReviewed does, and gave worldWith an adapters object.

---------

Co-authored-by: Mads Kronborg <mk@cego.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant