Repository navigation
Validate end-to-end, test the untested modules, ship as a gh extension - #1
Open
aditya-dua wants to merge 1 commit into
Open
aditya-dua wants to merge 1 commit into
aditya-dua wants to merge 1 commit into
Conversation
The pure functions were well covered, but `init`, `save`, `store.ts` and `stack.ts` had never run against real GitHub. Exercising them turned up one substantial defect and several smaller ones. Stack resolution never worked. `stack.ts` read a `stack` object from the pull request REST resource, but REST has no such field — `.stack` is absent on both the single and list endpoints, so every stack silently collapsed to a single layer. Stacks are only exposed via GraphQL (`PullRequest.stackEntry`), which also returns every member's position in one round-trip. GitHub further reports a native stack only for pull requests created through its stacking flow, so a hand-chained stack is now reconstructed by walking base-to-head links, stopping at any branch point since a tree is not a stack. Other fixes found by running the thing: - `gh api --paginate` concatenates one JSON array per page, which is not valid JSON; parsing now goes through `--jq '.[]'` and reads JSON lines. - Comment bodies went through `-f body=`, which mangles multi-line markdown. They are now sent as JSON on stdin via `--input -`. - A store created without `--add-readme` has an unborn default branch, so the first push could publish the local `init.defaultBranch` instead. - `capturedAt` moves every run, so `save` pushed a commit even when nothing changed. Records are now compared ignoring that field. - A fallback git identity was applied unconditionally; `-c user.email=` overrides configured values, so it is now used only when none is set. - `??` does not short-circuit on an explicit null, so a missing adapter was re-probed once per layer. Testing required a seam: `src/exec.ts` introduces a `CommandRunner` that every module taking `gh` or `git` now accepts, so the subprocess surface can be faked without a network. Tests run against source, which meant relative imports had to carry `.ts` extensions — `rewriteRelativeImportExtensions` restores `.js` on emit, so the published package is unchanged. Tests are now type-checked too, which immediately caught fixture drift from the real record shape. Finally, `gh extension install` requires the repository to be named `gh-*`, which this one is not. Rather than rename, `stacklore gh-install` writes the extension shim itself, so `gh stacklore ...` works for a global npm install or a source checkout alike. 62 new tests, 90 total. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.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
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.
The pure functions were well covered, but
init,save,store.tsandstack.tshad never run against real GitHub. I created a throwaway private source repo with a three-layer stack plus a standalone PR, and a throwaway private store, and ran the whole path. That turned up one substantial defect and several smaller ones.Stack resolution never worked
stack.tsread astackobject from the pull request REST resource. REST has no such field —.stackis absent on both the single-PR and list endpoints — sohead.stack.numberwas always null andgetStackearly-returned a single layer. Every stack silently collapsed to one layer, and no test caught it because nothing exercised the network path.Stacks are only exposed via GraphQL:
That also returns every member's position in one round-trip, so resolving a stack is now a single call.
GitHub further reports a native stack only for pull requests created through its stacking flow. A hand-chained stack — each PR targeting the one below — has
stackEntry: null. Those are now reconstructed by walking base-to-head links from the trunk, stopping at any branch point since a tree is not a stack, and guarded against cycles. Inferred layers carrysource: "inferred"so the distinction is never lost.Verified live:
getStackon PRs #1/#2/#3 of the scratch stack now returns#1@1/3 #2@2/3 #3@3/3, where previously each returned only itself.Other defects found by actually running it
gh api --paginateemits one JSON array per page, concatenated — not valid JSON.JSON.parsewould have thrown on any repo with more than 100 open PRs. Now--paginate --jq '.[]'and parsed as JSON lines.-f body=mangles multi-line markdown. The brief suspected this; it's correct. Comment bodies are now sent as JSON on stdin via--input -. Verified a table, backticks, quotes and emoji round-trip byte-for-byte.--add-readmehas an unborn default branch, so the clone has no HEAD and the first push could publish whateverinit.defaultBranchhappens to be locally rather than the repo's own.savewas not idempotent.capturedAtis regenerated every run, dirtying the tree and pushing a commit on every invocation. Records are now compared ignoring that field, using a key-order-independent serializer — key order matters because stored records are reparsed from disk.git -c user.email=…overrides configured values, so this would have hijacked a real identity. It is now supplied only when the machine genuinely has none. (This machine has none, which is how it got noticed.)??does not short-circuit on an explicitnull, so a missing adapter was re-probed once per layer.Also validated:
initon both the create and already-exists paths, a standalone PR correctly treated as a stack of one, and an empty session DB producing "nothing saved" rather than empty records.Testing the untested modules
Testing needed a seam.
src/exec.tsadds aCommandRunnerthat every module touchingghorgitaccepts, so the whole subprocess surface can be faked without a network. The fake throws on an unrouted command rather than returning empty output — which is how I caught three of my own tests that had forgotten to inject it and were quietly hitting the real API.62 new tests (90 total) across
stack.ts,store.ts,capture.tsand the new extension module, including the three behaviours called out in the brief:The re-probing test was initially vacuous, so I mutation-checked it: reverting the guard in
capture.tsdoes make it fail.Two incidental improvements fell out. Tests run against source, which required relative imports to carry
.tsextensions so Node's type stripping can resolve them — Node never rewrites.js→.ts, which is why no existing test had ever crossed a runtime import (distill.tsandredact.tsonly useimport type, which is erased).rewriteRelativeImportExtensionsrestores.json emit, so the published package is byte-identical in shape. And tests are now type-checked viatsconfig.test.json, which immediately caught a fixture that had drifted from the realCaptureSourceandRedactionReportshapes.Packaging as a gh extension
gh extension installrequires the repository to be namedgh-*; this one is namedstacklore. I verified the constraint empirically —gh extension install .on a directory namedstacklorefails withextension name must start with 'gh-'.Rather than rename the package,
stacklore gh-installwrites the extension shim itself into gh's extension directory, so it works for a global npm install, npx, or a source checkout alike. A bash shim rather than a symlink, because gh execs the file directly and a.jsfile has no interpreter of its own. Paths are JSON-quoted so a home directory with spaces can't split the command.Verified:
gh stacklore statusruns, flags pass through (gh stacklore list --json), reinstall is idempotent, and uninstall is clean.stacklore statusnow reports whether the extension is installed.A
gh-stacklorescript at the repo root is the entry point for a companiongh-stacklorerepository, documented as a follow-up.Verification
npm run typecheck,npm test(90 passing), andnpm run buildall green;node dist/cli.jsruns from a clean build.Cleanup needed
The throwaway repos could not be deleted — this token lacks the
delete_reposcope:Both are private. No new adapters were added, per the brief.