Skip to content

Commit b0b7b34

Browse files
committed
fix: address issues #153-#156 (App wf fallback, test hardening, ruleset strict=false, coverage tests)
- #153: auto-approve now falls back to GITHUB_TOKEN for auto-merge on wf-touching PRs (conditional App token creation); removes hard dependency on workflows:write grant. - #154: added normalize/CRLF test case exercising pure helper in initializeProject. - #155: bumped e2e managed/MCP timeouts to 60s for cold-start robustness. - #156: updated live ruleset strict_required_status_checks_policy=false; release PRs now merge reliably when green. Closes #153 Closes #154 Closes #155 Closes #156 All via npm run check (clean), double-checked, subagent reviewer passed (mostly ready, followed suggestions for full gate + draft PR flow). Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
1 parent 1f67502 commit b0b7b34

4 files changed

Lines changed: 29 additions & 17 deletions

File tree

.github/workflows/auto-approve.yml

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,26 +34,27 @@ jobs:
3434
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
3535
run: gh pr review --approve "${{ github.event.pull_request.number }}" --repo "${{ github.repository }}"
3636

37-
- uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
38-
if: github.event.pull_request.user.login != 'patchloom-release[bot]'
39-
id: app-token
40-
with:
41-
client-id: ${{ vars.APP_CLIENT_ID }}
42-
private-key: ${{ secrets.APP_PRIVATE_KEY }}
43-
44-
- name: Check for workflow file changes (to avoid App token needing workflows:write)
37+
# Check for workflow file changes early (to decide whether we can safely use App token for auto-merge)
38+
- name: Check for workflow file changes (use GITHUB_TOKEN fallback to avoid needing workflows:write on App)
4539
id: wf-changes
4640
env:
4741
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
4842
run: |
4943
pr="${{ github.event.pull_request.number }}"
5044
if gh pr view "$pr" --json files --jq '.files[].path' | grep -q '^\.github/workflows/'; then
5145
echo "changes=true" >> "$GITHUB_OUTPUT"
52-
echo "PR touches .github/workflows/; will skip auto-merge (App lacks workflows:write)"
46+
echo "PR touches .github/workflows/; will use GITHUB_TOKEN for auto-merge (no workflows:write needed on App)"
5347
else
5448
echo "changes=false" >> "$GITHUB_OUTPUT"
5549
fi
5650
51+
- uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0
52+
if: github.event.pull_request.user.login != 'patchloom-release[bot]' && steps.wf-changes.outputs.changes != 'true'
53+
id: app-token
54+
with:
55+
client-id: ${{ vars.APP_CLIENT_ID }}
56+
private-key: ${{ secrets.APP_PRIVATE_KEY }}
57+
5758
# Strong guard against accidental/automated release PR merges is also
5859
# implemented here (label check below) and via scripts/guard-no-release-merge.sh.
5960
# See AGENTS.md "Release PRs - Strong Guard" section.
@@ -72,11 +73,10 @@ jobs:
7273
echo "is_release_pr=false" >> "$GITHUB_OUTPUT"
7374
fi
7475
75-
- name: Enable auto-merge
76+
- name: Enable auto-merge (App token when no wf changes; GITHUB_TOKEN fallback otherwise)
7677
if: >-
7778
github.event.pull_request.user.login != 'patchloom-release[bot]' &&
78-
steps.wf-changes.outputs.changes != 'true' &&
7979
steps.release-guard.outputs.is_release_pr != 'true'
8080
env:
81-
GH_TOKEN: ${{ steps.app-token.outputs.token }}
81+
GH_TOKEN: ${{ steps.app-token.outputs.token || secrets.GITHUB_TOKEN }}
8282
run: gh pr merge --auto --squash "${{ github.event.pull_request.number }}" --repo "${{ github.repository }}"

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,12 @@ All I/O-dependent functions accept an `inputs` object with injectable callbacks
126126
- All relative imports must use `.js` extensions (`from "./foo.js"`, not `from "./foo"`). Required by `moduleResolution: "node16"`.
127127
- All commits require a `Signed-off-by` line (DCO). Use `git commit -s`.
128128
- When adding commands to `package.json`, update the expected count in `test/suite/index.ts`.
129+
- **Branch & PR workflow (never push a branch and stop):** For any trackable work,
130+
after the first `git push` immediately create a draft PR (`gh pr create --draft`).
131+
Continue development with normal `git push` (updates the draft PR + CI).
132+
Only run `gh pr ready <number>` (and enable auto-merge if needed) when the
133+
changes are ready for review/merge. This ensures every pushed branch is
134+
backed by an open (draft) PR from the start. See `~/.grok/skills/owned-repo-gate/SKILL.md`.
129135

130136
## Release PRs - Strong Guard
131137

test/unit/initializeProject.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ test("classifyAgentsFile detects real content drift", () => {
5353
assert.equal(classifyAgentsFile("# Rules\n- One\n", "# Rules\n- Two\n"), "different");
5454
});
5555

56+
test("normalizeForComparison (via classify) ignores trailing whitespace and normalizes line endings", () => {
57+
// indirectly exercises normalizeForComparison
58+
assert.equal(classifyAgentsFile("# Rules\n- One \n \n", "# Rules\n- One\n"), "up_to_date");
59+
assert.equal(classifyAgentsFile("# Rules\r\n- One\r\n", "# Rules\n- One\n"), "up_to_date");
60+
});
61+
5662
test("buildStatusDetails includes workspace readiness context", () => {
5763
const details = buildStatusDetails(
5864
{

test/unit/patchloomCli.test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -647,16 +647,16 @@ describe("managed install end-to-end MCP", { timeout: 120_000 }, async () => {
647647

648648
// Verify the binary is executable
649649
test("managed install produces a runnable binary", async () => {
650-
// Cold start after fresh managed install can take >15s on some runners; use 30s.
651-
const { stdout, stderr } = await execFileAsync(binaryPath, ["--version"], { timeout: 30000 });
650+
// Cold start after fresh managed install can take >30s on some runners/CI; use 60s for robustness.
651+
const { stdout, stderr } = await execFileAsync(binaryPath, ["--version"], { timeout: 60000 });
652652
const output = `${stdout}${stderr}`.trim();
653653
const version = parsePatchloomVersion(output);
654654
assert.ok(version, `should parse version from managed binary: ${output}`);
655655
assert.match(version, /^\d+\.\d+\.\d+/);
656656
});
657657

658658
test("MCP server responds to initialize", async () => {
659-
const child = execFile(binaryPath, ["mcp-server"], { timeout: 15000 });
659+
const child = execFile(binaryPath, ["mcp-server"], { timeout: 60000 });
660660
let stdout = "";
661661
child.stdout!.on("data", (data: Buffer) => { stdout += data.toString(); });
662662

@@ -690,7 +690,7 @@ describe("managed install end-to-end MCP", { timeout: 120_000 }, async () => {
690690
});
691691

692692
test("MCP server lists available tools", async () => {
693-
const child = execFile(binaryPath, ["mcp-server"], { timeout: 15000 });
693+
const child = execFile(binaryPath, ["mcp-server"], { timeout: 60000 });
694694
let stdout = "";
695695
child.stdout!.on("data", (data: Buffer) => { stdout += data.toString(); });
696696

@@ -765,7 +765,7 @@ describe("managed install end-to-end MCP", { timeout: 120_000 }, async () => {
765765
const workDir = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-mcp-call-"));
766766
await fs.writeFile(path.join(workDir, "config.json"), '{"port": 3000}\n', "utf8");
767767

768-
const child = execFile(binaryPath, ["mcp-server"], { timeout: 15000, cwd: workDir });
768+
const child = execFile(binaryPath, ["mcp-server"], { timeout: 60000, cwd: workDir });
769769
let stdout = "";
770770
child.stdout!.on("data", (data: Buffer) => { stdout += data.toString(); });
771771

0 commit comments

Comments
 (0)