Skip to content

fix(setup): known-agent-login flag, merge method detection, truncation warnings, drop dead runChecks - #60

Merged
yshyn-iohk merged 2 commits into
mainfrom
claude/setup-gaps
Aug 21, 2026
Merged

fix(setup): known-agent-login flag, merge method detection, truncation warnings, drop dead runChecks#60
yshyn-iohk merged 2 commits into
mainfrom
claude/setup-gaps

Conversation

@yshyn-iohk

Copy link
Copy Markdown
Collaborator

Summary

Implements the items from issue #55 that fall inside this PR's file boundary,
plus the documentation half of issue #54 (the security-alert rail's scoping
logic is a separate change).

Fixed (issue #55)

  • Item 1: init gains --known-agent-login <login...> (repeatable),
    mirroring --reviewer. It writes knownAgentLogins to config.json and the
    printed summary now names the resulting list, since AGENTS.md calls this
    field "the easy thing to forget."
  • Item 2: pr_expedite/pr_approve_dep_upgrade (pi/src/extension.ts)
    now resolve a merge method when a call omits one and autonomy: "auto" is
    given, instead of always falling through to the operations' own "merge"
    default: an explicit per-call mergeMethod still wins outright, then a new
    per-repo config.mergeMethodByRepo setting, then a best-effort read of the
    repository's own allowed merge methods via a new, optional
    GitHubGateway.getAllowedMergeMethods (implemented on OctokitGateway,
    added as an optional interface member so no existing fake gateway needs to
    grow one). A repository restricted to squash-only or rebase-only merges no
    longer 405s on every auto-merge attempt. The probe only runs when autonomy
    is "auto", so the common propose-only path gets no extra API call.
  • Item 4: all three discover.mjs scripts (pr-requester, pr-reviewer,
    pr-steward), and their byte-identical .pi/taskflows copies, now warn on
    stderr when a gh call returns exactly the --limit 100 cap, since gh
    truncates silently past it with no other signal.
  • Item 5: runChecks is removed from ConfigSchema (core/model.ts) and
    from the docs (docs/quick-start.md, docs/cli.md, SECURITY.md), rather
    than implemented: no non-test code path ever read it, which SECURITY.md
    already admitted. Schemas regenerated via npm run gen:schemas and
    committed. SECURITY.md keeps a short note recording the removal instead of
    silently dropping the sentence.
  • Item 6: pi/src/extension.ts now memoizes the default OctokitGateway
    per process (built at most once, lazily, on first use) via a small once()
    helper, instead of the previous deps.gh ?? (() => new OctokitGateway())
    building a brand-new client, with a cold ETag cache and no cached
    authenticated login, on every single tool call. An injected deps.gh (every
    test, and any future caller with its own lifecycle) is unaffected.
  • Item 7: docs/quick-start.md now says, next to captureMetadata in the
    config table, that the dashboard's Agents and Collaborators views need it on
    for per-agent attribution, and what happens without it (a single "Unknown"
    row).

Fixed (issue #54, documentation and setup half)

  • Documents the permission the security-alert rail needs beyond the base
    recommended scope, Dependabot alerts: read (fine-grained) /
    security_events (classic), in SECURITY.md (new "Additional scope for
    expedition auto mode" section) and docs/quick-start.md.
  • init now makes one best-effort, read-only probe of the Dependabot alerts
    endpoint (against the first --repo only, since the permission is a
    property of the token, not of any one repository) and returns an unmissable
    warning, printed to stderr surrounded by ! rules, when it cannot read that
    endpoint. The probe is best-effort by construction: any thrown error is
    treated the same as the gateway's own "cannot tell" (null) result, so a
    probe failure never fails init itself. The wording is explicit that this
    permission is unrelated to requesting, claiming, or completing a review, and
    only gates the expedition taskflows' autonomy=auto path.

Explicitly left alone

Test plan

  • Root: typecheck, typecheck:test, check:schemas, check:version,
    test (629 passed), build all green.
  • -w pi: typecheck, test (30 passed), build all green.
  • -w dashboard build green.
  • cd docs && npm run build green.
  • node --check on each edited discover.mjs.
  • New tests: init --known-agent-login passthrough and summary naming,
    the security-alerts probe (warns on null, warns on a thrown error but never
    rejects, stays silent on a readable count, checks only the first repo), the
    once() memoization primitive, and merge-method resolution (repository
    probe, per-repo config precedence over the probe, explicit mergeMethod
    winning over both, and no probe at all in propose mode).

🤖 Generated with Claude Code

…n warnings, drop dead runChecks

Closes several setup and defaults gaps from issue #55, and the documentation
half of issue #54:

- init gains --known-agent-login (mirrors --reviewer) and names knownAgentLogins
  in its printed summary.
- pr_expedite/pr_approve_dep_upgrade resolve a repository's allowed merge
  methods (new OctokitGateway.getAllowedMergeMethods) or a configured
  mergeMethodByRepo entry when a call omits mergeMethod, instead of always
  falling back to "merge" and 405ing on a squash-only or rebase-only repo.
- All three discover.mjs scripts (and their .pi/taskflows mirrors) warn on
  stderr when a gh call returns exactly the --limit cap, since gh truncates
  silently past it.
- runChecks is removed from the config schema and docs: no non-test code ever
  read it, per SECURITY.md's own admission. Schemas regenerated.
- pi/src/extension.ts memoizes the default OctokitGateway per process so its
  ETag cache and cached login survive across tool calls in a run.
- docs/quick-start.md now says where a dashboard user will read it that
  captureMetadata must be on for per-agent attribution.
- Documents the Dependabot alerts permission (issue #54) in SECURITY.md and
  docs/quick-start.md, and init now makes a best-effort probe of that endpoint,
  warning when autonomy=auto could never fire without it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Yurii Shynbuiev <yurii.shynbuiev@iohk.io>
@yshyn-iohk
yshyn-iohk enabled auto-merge (squash) August 21, 2026 05:36
@yshyn-iohk
yshyn-iohk merged commit ee0033c into main Aug 21, 2026
7 checks passed
@yshyn-iohk
yshyn-iohk deleted the claude/setup-gaps branch August 21, 2026 05:37
yshyn-iohk added a commit that referenced this pull request Aug 21, 2026
…ional (#64)

Stacked on #60, which removed `runChecks` from the schema. This finishes
the job: the field was still described in six places after the removal.

The two that mattered are the skill files. Both told the reviewing agent
not to run the repository's build or test scripts **"unless `runChecks`
is enabled"**, which reads as though there is a way to enable it. There
is not, and there should not be: the diff under review is untrusted
input, so executing it is out of scope for a review. The rule is now
unconditional, which is both accurate and stronger than what it
replaced.

The rest:
- `docs/files-and-directories.md` listed it as a config field; it now
lists `reviewers` and `knownAgentLogins` instead, which do exist.
- `examples/config.json` shipped it as an example value; the example now
shows the fields a real setup needs, including the `knownAgentLogins`
that three other documents call the easy one to forget.
- ADR 0007 keeps its original decision text and gains a dated note. An
ADR records a decision that was made, so rewriting it would be
dishonest; the decision did not change, only the mechanism that was
supposed to gate it.
- Six test fixtures still passed the dead field. Harmless (zod strips
unknown keys), but a reader grepping for it would reasonably conclude it
exists.

After this, the only remaining mention in the repository is the
deliberate historical note in `SECURITY.md` and the ADR note above.

## Verification
629 tests green, plus typecheck, `typecheck:test`, `check:schemas`, and
`check:version` (0.5.0, no bump). No behavior change outside the skill
instruction, which is documentation the agent follows.

---------

Signed-off-by: Yurii Shynbuiev <yurii.shynbuiev@iohk.io>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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