Skip to content

chore(loop): weekly retro schedule, credential guard, dependabot CI fix - #37

Merged
Alex1990 merged 3 commits into
mainfrom
chore/loop-a1-followups
Sep 10, 2026
Merged

Alex1990 merged 3 commits into
mainfrom
chore/loop-a1-followups

Conversation

@Alex1990

Copy link
Copy Markdown
Owner

Three fixes that landed after #36 was merged — they were pushed to the branch a minute too late to be included, so they are re-applied here on top of main. Nothing in #36 is reverted or rewritten; this is the tail of that work.

1. Wire the weekly retro schedule

The router already distinguished the weekly cron, but the workflow only declared the daily one, so retro-scheduled could never fire — the weekly self-optimisation pass was dead code:

- cron: '17 3 * * *'   # daily sweep
- cron: '23 3 * * 1'   # weekly retro (Mondays 03:23 UTC)

The comment records why the string must match WEEKLY_CRON exactly: the router compares the cron value with equality, so a typo would silently run a sweep instead of failing loudly. Verified by parsing the cron values back out of the YAML and feeding them to the real router (daily → sweep, weekly → retro-scheduled), and by confirming that stage maps to skills/retro/SKILL.md.

2. Guard pull_request runs that have no secrets

pull_request is the only event we listen to where secrets can be absent — GitHub treats a Dependabot-triggered run like a fork run (read-only token, no secrets at all), and a fork PR never has them. Unguarded, such a run dies at Pull state from R2 and shows up as a red loop: indistinguishable from a real defect, while it cannot do anything useful anyway.

if: >-
  github.event_name != 'pull_request'
  || (github.event.pull_request.head.repo.full_name == github.repository
      && github.actor != 'dependabot[bot]')

Verified across 12 event shapes: every event carrying secrets still runs, Dependabot and fork PRs skip, and a missing head repository falls back to skipping.

Consequence, accepted deliberately for A1 (recorded as D28): the router's deps and external-PR readonly rows cannot actually run, because even a read-only analysis needs the LLM key. Enabling them later means giving that path its own credentials (pull_request_target two-step per host spec §6, or Dependabot secrets). The trigger table in scripts/loop/README.md now marks both rows as unrunnable so the docs match reality.

3. Fix the Dependabot red CI (D21)

Comment coverage on PR failed with Resource not accessible by integration (HTTP 403): GitHub issues a read-only GITHUB_TOKEN to Dependabot-triggered runs. The test job now grants pull-requests: write, so a Dependabot PR no longer reports a failed CI whose tests all passed.

Correction worth noting: I earlier claimed in #36 that permissions: could not lift this. That is the fork rule, not the Dependabot one — the official docs name permissions as the fix for Dependabot runs. The defect log has been corrected rather than quietly deleted.

Also

4691a27-equivalent: a stale path in the cron comment still pointed at scripts/loop/lib/route.mjs after the D10 rename to shared/.

actionlint clean on both workflows; local regression suite (route decisions 17/17, failure classification 9/9, CLI flow with its guards, all orchestrator paths) still green.

The comment pointed at `scripts/loop/lib/route.mjs`; the shared library
moved to `scripts/loop/shared/` in D10. Caught while reviewing the
trigger table.
The router already distinguished the weekly cron, but the workflow only
declared the daily one, so `retro-scheduled` could never fire — the weekly
self-optimisation pass was dead code. Added `23 3 * * 1` (Mondays, 03:23 UTC).

The comment records why the string must match `WEEKLY_CRON` exactly: the
router compares the cron value with equality, so a typo here would silently
run a sweep instead of failing loudly. Verified by parsing the cron values
back out of the workflow and feeding them to the real router — daily resolves
to `sweep`, weekly to `retro-scheduled` — and by confirming the stage maps to
`skills/retro/SKILL.md`.
Two related credential problems on `pull_request`.

D21 — the dependabot red CI. `Comment coverage on PR` failed with
"Resource not accessible by integration (HTTP 403)": GitHub issues a
read-only GITHUB_TOKEN to Dependabot-triggered runs, which it treats like
fork runs. An earlier note in the defect log claimed `permissions:` could
not help; that is the **fork** rule, not the Dependabot one — the official
docs name `permissions` as the fix. The `test` job now grants
`pull-requests: write`, so a Dependabot PR no longer reports a failed CI
whose tests all passed.

D28 — runs that cannot authenticate at all. Dependabot-triggered runs and
fork PRs get **no secrets**, not merely no write permission, so even the
read-only external-PR analysis (which needs the LLM key) and `deps` (which
needs R2) are impossible in that context. The loop job now skips them, so
they are visibly skipped instead of failing at `Pull state from R2` and
looking like a real defect. `pull_request_target`, `issues`, `schedule`,
`release` and `workflow_dispatch` keep their secrets and still run.

Verified the guard's truth table across 12 event shapes: every event with
secrets runs, Dependabot and fork PRs skip, and a missing head repository
falls back to skipping.
@github-actions

Copy link
Copy Markdown
Contributor

Coverage

Metric % Covered / Total
Lines 90.19% 1003/1112
Statements 89.06% 1083/1216
Functions 91.03% 193/212
Branches 76.43% 519/679

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant