Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion plugins/pstack/skills/poteto-mode/playbooks/orchestrate.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ A dependency is a context relay, not just ordering. Undeclared upstream context

#### Stack safety

- The frontier is a computed object, never narrative. Recompute `frontier.json` from `gt` after every merge and stack mutation because GitHub base refs drift mid-restack while gt tracking is authoritative: ordered PR list, branch names, head SHAs, a generation number, the lowest unmerged PR. Resolve it where gt knows the stack, normally the stacker's clone. A checkout whose gt metadata never saw the submits reports no PRs and the command errors rather than guessing.
- The frontier is a computed object, never narrative. Recompute `frontier.json` after every merge and stack mutation because GitHub base refs drift mid-restack. `gt` supplies the authoritative stack order and branch mapping; `gh` supplies each PR's live open, merged, or closed state because `gt info` can return cached state while its background refresh is still running. Record the ordered PR list, branch names, head SHAs, a generation number, and the lowest unmerged PR. Resolve it from a branch in the target linear stack, normally in the stacker's clone. Do not resolve from a trunk with multiple tracked child stacks because `gt log --stack` returns a branching graph and `orch` refuses to guess a chain. A checkout whose gt metadata never saw the submits reports no PRs and the command errors rather than guessing.
- Exactly one stacker per stack may run `gt`, serialized within its stack. Record the holder in the standing orders. A restack at this scale is slow and blocks whoever runs it, so give it its own unit and keep the coordinator out of it.
- Workers never rebase and never run `gt`. Babysitters follow `playbooks/babysit.md`, one per stack, scoped to one immutable frontier generation. They report conflicts to the stacker rather than restacking.
- PR closes and retargets go through the stacker only. Closing a base PR orphans every chain above it. Merges and stack surgery are units with briefs like any other.
Expand Down
134 changes: 125 additions & 9 deletions plugins/pstack/skills/poteto-mode/scripts/orch/orch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ interface RunResult {
readonly stderr: string;
}

interface FakeCliPaths {
readonly gh: string;
readonly gt: string;
readonly gtOutput: string;
}

async function makeDirectory(): Promise<string> {
const directory = await mkdtemp(join(tmpdir(), "orch-test-"));
directories.push(directory);
Expand Down Expand Up @@ -82,6 +88,15 @@ async function makeGitStack(directory: string): Promise<{
git({ repo, args: ["init", "--initial-branch=main"] });
git({ repo, args: ["config", "user.name", "Orch Test"] });
git({ repo, args: ["config", "user.email", "orch@example.com"] });
git({
repo,
args: [
"remote",
"add",
"origin",
"https://github.com/contributor/widgets.git",
],
});
await writeFile(join(repo, "main.txt"), "main\n");
git({ repo, args: ["add", "."] });
git({ repo, args: ["commit", "-m", "main"] });
Expand All @@ -102,13 +117,13 @@ async function makeGitStack(directory: string): Promise<{
};
}

async function withFakeGt<T>({
async function withFakeGtAndGh<T>({
directory,
operation,
output,
}: {
directory: string;
operation: (outputPath: string) => Promise<T>;
operation: (paths: FakeCliPaths) => Promise<T>;
output: string;
}): Promise<T> {
const bin = join(directory, "bin");
Expand All @@ -129,13 +144,13 @@ case "$*" in
cat "${outputPath}"
;;
"--no-interactive info stack/merged")
printf 'stack/merged\\nPR #10 (Merged) merged change\\n'
printf 'stack/merged\\nPR #10 (New Graphite status) merged change\\nhttps://app.graphite.com/github/pr/base-owner/widgets/10\\ncommit body with an unrelated link\\nhttps://app.graphite.com/github/pr/other-owner/other-repo/99\\n'
;;
"--no-interactive info stack/closed")
printf 'stack/closed\\nPR #13 (Closed) closed change\\n'
printf 'stack/closed\\nPR #13 (Closed) closed change\\nhttps://app.graphite.com/github/pr/base-owner/widgets/13\\n'
;;
"--no-interactive info stack/open")
printf 'stack/open\\nPR #11 (Needs approvals) open change\\n'
printf 'stack/open\\nPR #11 (Needs approvals) open change\\nhttps://app.graphite.com/github/pr/base-owner/widgets/11\\n'
;;
*)
printf 'unexpected gt arguments: %s\\n' "$*" >&2
Expand All @@ -146,10 +161,34 @@ esac
);
await chmod(gt, 0o755);

const gh = join(bin, "gh");
await writeFile(
gh,
`#!/usr/bin/env bash
set -euo pipefail
case "$*" in
"pr view 10 --repo github.com/base-owner/widgets --json state --jq .state")
printf 'MERGED\\n'
;;
"pr view 13 --repo github.com/base-owner/widgets --json state --jq .state")
printf 'CLOSED\\n'
;;
"pr view 11 --repo github.com/base-owner/widgets --json state --jq .state")
printf 'OPEN\\n'
;;
*)
printf 'unexpected gh arguments: %s\\n' "$*" >&2
exit 2
;;
esac
`
);
await chmod(gh, 0o755);

const originalPath = process.env.PATH;
process.env.PATH = `${bin}:${originalPath ?? ""}`;
try {
return await operation(outputPath);
return await operation({ gh, gt, gtOutput: outputPath });
} finally {
if (originalPath === undefined) {
delete process.env.PATH;
Expand Down Expand Up @@ -406,7 +445,7 @@ describe("Store", () => {
]);
});

it("resolves the ordered Graphite frontier and validates an optional pin", async () => {
it("uses adjacent PR metadata despite an unknown status and later PR link", async () => {
const { directory, store } = await initializedStore();
const stack = await makeGitStack(directory);
const output = `◯ main
Expand All @@ -415,7 +454,7 @@ describe("Store", () => {
◉ stack/open (current)
`;

await withFakeGt({
await withFakeGtAndGh({
directory,
output,
operation: async () => {
Expand Down Expand Up @@ -478,11 +517,88 @@ describe("Store", () => {
});
});

it("preserves the previous frontier when GitHub state lookup fails", async () => {
const { directory, store } = await initializedStore();
const stack = await makeGitStack(directory);

await withFakeGtAndGh({
directory,
output: "◯ main\n◉ stack/merged\n",
operation: async ({ gh }) => {
const before = await store.frontier.set({ repo: stack.repo });
await writeFile(gh, "#!/usr/bin/env bash\nexit 1\n");

await expect(
store.frontier.set({ repo: stack.repo })
).rejects.toThrow("gh pr view 10 failed for branch stack/merged");
expect(await store.frontier.show()).toEqual(before);
},
});
});

it("preserves the previous frontier when GitHub returns an invalid state", async () => {
const { directory, store } = await initializedStore();
const stack = await makeGitStack(directory);

await withFakeGtAndGh({
directory,
output: "◯ main\n◉ stack/merged\n",
operation: async ({ gh }) => {
const before = await store.frontier.set({ repo: stack.repo });
await writeFile(gh, "#!/usr/bin/env bash\nprintf 'UNKNOWN\\n'\n");

await expect(
store.frontier.set({ repo: stack.repo })
).rejects.toThrow(
"gh pr view 10 returned an invalid state for branch stack/merged"
);
expect(await store.frontier.show()).toEqual(before);
},
});
});

it("preserves the previous frontier when Graphite PR identity mismatches", async () => {
const { directory, store } = await initializedStore();
const stack = await makeGitStack(directory);

await withFakeGtAndGh({
directory,
output: "◯ main\n◉ stack/merged\n",
operation: async ({ gt, gtOutput }) => {
const before = await store.frontier.set({ repo: stack.repo });
await writeFile(
gt,
`#!/usr/bin/env bash
set -euo pipefail
case "$*" in
"--no-interactive log short --stack --reverse")
cat "${gtOutput}"
;;
"--no-interactive info stack/merged")
printf 'stack/merged\\nPR #10 (Ready to merge) change\\nhttps://app.graphite.com/github/pr/base-owner/widgets/12\\n'
;;
*)
exit 2
;;
esac
`
);

await expect(
store.frontier.set({ repo: stack.repo })
).rejects.toThrow(
"gt info output PR identity mismatch for branch stack/merged: row 10, URL 12"
);
expect(await store.frontier.show()).toEqual(before);
},
});
});

it("rejects unparseable Graphite output loudly", async () => {
const { directory, store } = await initializedStore();
const stack = await makeGitStack(directory);

await withFakeGt({
await withFakeGtAndGh({
directory,
output: "◯ main\nthis line is not Graphite output\n",
operation: async () => {
Expand Down
Loading