Skip to content

feat(github-scan): allow declaring agent identity separately from operator (#360) - #361

Merged
serenakeyitan merged 3 commits into
mainfrom
feat/360-agent-login
May 3, 2026
Merged

feat(github-scan): allow declaring agent identity separately from operator (#360)#361
serenakeyitan merged 3 commits into
mainfrom
feat/360-agent-login

Conversation

@serenakeyitan

Copy link
Copy Markdown
Contributor

Refs #360. Surfaced from the live #359 smoke (test report) where the operator's gh auth user (serenakeyitan) was the same as the daemon's agent identity, so every operator comment was filtered as own-comment and auto-revert never fired.

Summary

Adds a --agent-login <login> CLI flag (with env and config-file fallbacks) so the operator's GitHub identity can be declared independently of the daemon's gh auth user. The auto-revert own-comment guard from #358/#359 now uses this resolved identity.

Resolution order

# Source Wins over
1 CLI flag --agent-login <login> env, yaml, gh auth
2 Env var GITHUB_SCAN_AGENT_LOGIN yaml, gh auth
3 Yaml agent_login / agentLogin (in ~/.first-tree/github-scan/config.yaml) gh auth
4 Daemon's gh auth identity (final fallback — preserves zero-config dogfood)

Implementation

  • runtime/config.ts: adds agentLogin to DaemonConfig, DaemonCliOverrides, the yaml schema, and the existing 4-tier loadGitHubScanDaemonConfig resolver.
  • daemon/runner-skeleton.ts: parses --agent-login flag (both --agent-login alt-bot and --agent-login=alt-bot), passes config.agentLogin ?? identity?.login into runPoller / runPollerOnce. Logs an explicit override line when the resolved identity differs from gh auth.
  • cli.ts: surfaces --agent-login <login> in run, daemon, and start help text.
  • auto-revert.ts already takes agentLogin as a parameter (PR fix(github-scan): auto-revert github-scan:human on new human comment (#358) #359), so no change needed there — the resolved value just flows through.

Tests

  • 6 new config tests (all 4 resolution tiers + camelCase yaml + empty-CLI-no-clobber)
  • 2 new parseDaemonArgs tests (both --agent-login forms + empty-value rejection)
  • 1 new auto-revert test verifying guard 1 (own-comment) uses the resolved identity, NOT the gh auth user — this is the exact feat(github-scan): allow declaring agent identity separately from operator (--agent-login) #360 scenario
  • All existing 11 auto-revert tests + existing config tests still pass
  • pnpm -r test (531 tests), pnpm lint, pnpm typecheck all pass

Live smoke (3 cases — all green)

Tested against agent-team-foundation/first-tree-website#12 on a separate daemon (port 7879, separate GITHUB_SCAN_DIR) without disturbing the running prod daemon on 7878. Full transcript posted as a follow-up comment.

Case Setup Auto-revert expected Result
1 --agent-login some-other-login YES (operator comment is "human" now) YES — label stripped, status=new
2 GITHUB_SCAN_AGENT_LOGIN=some-other-login, no flag YES YES — label stripped
3 No flag, no env NO (gh auth fallback → serenakeyitan IS the agent → own-comment filter) NO — label retained

cc @bingran-you

serenakeyitan and others added 2 commits May 1, 2026 17:58
When the daemon polls an item classified `human` and observes a
qualifying human comment posted strictly after the
`github-scan:human` label was applied, strip the label so the
classifier naturally re-derives `new` on the next cycle and the
dispatcher picks the item up.

Guards (issue #358):
  1. Comment author MUST NOT be the agent itself.
  2. Comment body length > 20 chars (filters thumbs-up / "ok" acks).
  3. Reactions alone do NOT count as a comment.
  4. Comment created_at MUST be strictly after the label-event timestamp.

The fix is a new `runtime/auto-revert.ts` module wired into
`pollOnce` between `enrichWithLabels` and `classifyEntries`. The
classifier itself is unchanged. Production passes the daemon's
resolved `identity.login` as `agentLogin` so the own-comment guard
is exact; when identity resolution failed (degraded mode), the
auto-revert is skipped entirely.

Updated the agent prompt in `daemon/runner.ts` to document the new
human → new transition so agents know they can safely stop on
`github-scan:human` and trust the daemon to re-queue on reply.

Refs #358

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rator (#360)

Adds a `--agent-login <login>` CLI flag (with `GITHUB_SCAN_AGENT_LOGIN`
env var and `agent_login` config-file fallbacks) so the operator's
GitHub identity can be declared independently of the daemon's `gh auth`
user. The auto-revert own-comment guard from #358/#359 now uses this
resolved identity instead of always falling back to `gh auth whoami`.

Resolution order (highest wins):
  1. CLI flag `--agent-login`
  2. Env var `GITHUB_SCAN_AGENT_LOGIN`
  3. Config-file key `agent_login` / `agentLogin`
  4. Fallback: daemon's `gh auth` identity (preserves zero-config dogfood)

Touch points (matches issue #360):
  - `runtime/config.ts`: adds `agentLogin` to `DaemonConfig`,
    `DaemonCliOverrides`, yaml schema, and 4-tier resolution.
  - `daemon/runner-skeleton.ts`: parses `--agent-login` flag, plumbs
    `config.agentLogin ?? identity?.login` through to `runPoller` and
    `runPollerOnce`. Logs an explicit override line when the resolved
    identity differs from `gh auth`.
  - `cli.ts`: surfaces `--agent-login <login>` in `run`, `daemon`, and
    `start` help text.

Tests:
  - 6 new config tests covering all 4 resolution tiers + empty-value
    no-clobber.
  - 2 new parseDaemonArgs tests covering both --agent-login forms and
    empty-value rejection.
  - 1 new auto-revert test verifying guard 1 (own-comment) uses the
    resolved identity, NOT the gh auth user (the exact scenario that
    blocked the live #359 smoke).
  - All existing 11 auto-revert tests + 31 config tests still pass.
  - pnpm -r test (531 tests), pnpm lint, pnpm typecheck all pass.

Refs #360.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@serenakeyitan
serenakeyitan requested a review from bingran-you May 2, 2026 01:34
@serenakeyitan

Copy link
Copy Markdown
Contributor Author

Live end-to-end smoke — issue #360

Modified daemon, port 7879, GITHUB_SCAN_DIR=/tmp/scan-test-pr360, --dry-run so the test daemon never actually dispatches Claude/Codex on the test target. Production daemon on PID 50451 / port 7878 was left untouched throughout.

Test target: agent-team-foundation/first-tree-website#12 (PR; github-scan:human was applied/stripped repeatedly during the run, ended at github-scan:done).

Case 1 — CLI flag (--agent-login some-other-login) → auto-revert SHOULD fire

$ ... --agent-login some-other-login --dry-run
github-scan daemon: identity=serenakeyitan@github.com
github-scan daemon: agent-login overridden to 'some-other-login' (gh auth=serenakeyitan)
github-scan daemon: ... agent-login=some-other-login

Setup at 2026-05-02T01:28:23Z:

After ~50s (one poll cycle):

github-scan: polled 129 notifications (5 new)
WARN: auto-reverted github-scan:human on 1 item(s) (issue #358)
github-scan: polled 129 notifications (6 new)

Verification:

$ gh pr view 12 -R first-tree-website --json labels
{"labels":[]}                                            # label stripped ✓

$ curl -s :7879/inbox | jq '.notifications[] | select(.number==12)'
"github_scan_status": "new"                              # reclassified ✓

PASS.

Case 2 — Env var (GITHUB_SCAN_AGENT_LOGIN=some-other-login, no flag) → auto-revert SHOULD fire

Restarted daemon with no --agent-login flag; only GITHUB_SCAN_AGENT_LOGIN=some-other-login:

github-scan daemon: agent-login overridden to 'some-other-login' (gh auth=serenakeyitan)
github-scan daemon: ... agent-login=some-other-login

Setup at 2026-05-02T01:29:35Z:

After one poll cycle:

WARN: auto-reverted github-scan:human on 1 item(s) (issue #358)
$ gh pr view 12 -R first-tree-website --json labels
{"labels":[]}                                            # label stripped ✓

PASS.

Case 3 — No flag, no env → must NOT fire (gh auth fallback path, regression check for #359)

Restarted daemon with no flag, no env var, GITHUB_SCAN_AGENT_LOGIN unset (env -u):

github-scan daemon: identity=serenakeyitan@github.com
github-scan daemon: ... agent-login=serenakeyitan          # fallback to gh auth ✓

Note the absence of the "agent-login overridden" line — the resolved identity matches gh auth.

Setup at 2026-05-02T01:30:19Z:

After two poll cycles (60s+):

github-scan: polled 129 notifications (5 new)
candidates: submitted 7 task(s)
github-scan: polled 129 notifications (5 new)
                                                          # no auto-reverted line ✓
$ gh pr view 12 -R first-tree-website --json labels
{"labels":[{"name":"github-scan:human", ...}]}            # label retained ✓

PASS — the original PR #359 single-account-dogfood behaviour is preserved when no override is configured.

Cleanup


Result: 3/3 cases PASS. Resolution order verified end-to-end.

@bingran-you bingran-you added the breeze:wip breeze is actively working on it label May 2, 2026

@bingran-you bingran-you left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a few blocking issues before this is safe to merge:

  1. packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts:249-251 mutates entry.labels and records a successful revert immediately after calling GhClient.removeLabel, but removeLabel in packages/github-scan/src/github-scan/engine/runtime/gh.ts:169-184 intentionally swallows every gh failure. A transient API failure or permission problem will still reclassify the thread to new locally and let the dispatcher pick it up even though GitHub still still shows github-scan:human. This path needs an acknowledged success signal before mutating inbox state.

  2. fetchIssueComments and fetchHumanLabelAppliedAt only fetch a single per_page=100 page (packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts:97-100 and 143-149). On any busy issue/PR with more than 100 comments or timeline events, the qualifying reply or the most recent github-scan:human application can fall off the fetched page, so auto-revert will miss real replies or compare against a stale label timestamp. This needs pagination (or an equivalent latest-since-X strategy) before the feature is reliable.

  3. The new auto-revert only inspects /issues/{n}/comments (packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts:92-126). PR review bodies and review comments never go through that endpoint, so a reviewer can reply on a PR and the thread will stay stuck in github-scan:human. The rest of the daemon already treats reviews as visible activity (packages/github-scan/src/github-scan/engine/daemon/gh-client.ts:226-267), so the revert path should cover the same surfaces.

This reply was drafted by breeze, an autonomous agent running on behalf of the account owner.

# Conflicts:
#	packages/github-scan/src/github-scan/engine/daemon/runner-skeleton.ts
#	packages/github-scan/src/github-scan/engine/daemon/runner.ts
#	packages/github-scan/src/github-scan/engine/runtime/auto-revert.ts
#	packages/github-scan/tests/github-scan/github-scan-auto-revert.test.ts
@serenakeyitan

Copy link
Copy Markdown
Contributor Author

Updated branch with main (merge commit, no force-push). The blockers from your earlier review (inherited from #359) are now addressed in their own PRs:

This PR (#360 / --agent-login) is independent of those concerns — it only touches identity resolution (config.ts, runner-skeleton.ts, cli.ts) and adds a focused test for guard 1 using the resolved agent identity.

Conflicts resolved:

Gates after merge:

  • pnpm -r test: 559 passed (468 github-scan + 91 cli)
  • pnpm lint: clean
  • pnpm typecheck: clean

Live E2E was previously verified working in #361. Re-running was scoped down since the unit test for guard-1-with-resolved-identity (the exact #359 smoke failure scenario) passes against main's updated auto-revert.ts.

cc @bingran-you — ready for re-review.

Refs #360.

@serenakeyitan
serenakeyitan requested a review from bingran-you May 3, 2026 01:52
@bingran-you bingran-you added breeze:wip breeze is actively working on it and removed breeze:done breeze has finished handling it labels May 3, 2026

@bingran-you bingran-you left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review after main merge — looks good to ship.

Verified the three blockers from my prior review:

  1. removeLabel error-check — addressed on main (auto-revert.ts:361-368): the call now checks removed and only mutates entry.labels / pushes to reverted on success, with a will retry next cycle warning otherwise. ✓
  2. Pagination — addressed on main via #370: fetchIssueComments (auto-revert.ts:145-190) walks newest-first with an early-exit at labelAppliedAt, and fetchHumanLabelAppliedAt (auto-revert.ts:243-279) walks all timeline pages tracking the latest hit. Both capped at AUTO_REVERT_MAX_PAGES with warn-on-cap. ✓
  3. PR-review coverage — tracked separately in open PR #369. Fair scope boundary — this PR is purely identity resolution and does not regress that surface. ✓

This PR itself is well-scoped:

  • 4-tier resolution (CLI > env > yaml > gh-auth) is symmetric with the other config knobs and well-tested across all four tiers (github-scan-daemon-config.test.ts:271-330).
  • Both --agent-login alt-bot and --agent-login=alt-bot parse correctly; empty values are dropped (guards against --agent-login "" accidentally clobbering env/yaml).
  • The new auto-revert test (github-scan-auto-revert.test.ts:165-184) precisely captures the #360 scenario: gh-auth is serenakeyitan, resolved identity is first-tree-bot, operator comment must NOT be filtered as own-comment. This is exactly what the live #359 smoke missed.
  • Override log line is a nice operational signal.

Non-blocking nit: in runner-skeleton.ts, resolvedAgentLogin is computed at line 412 but the same config.agentLogin ?? identity?.login expression is re-typed at lines 588 and 613. Pulling those onto the local would prevent future drift if the resolution ever grows a third arm. Not worth holding the PR for.

Approving. Live smoke (3/3) and unit coverage are both convincing.

This reply was drafted by breeze, an autonomous agent running on behalf of the account owner.

@bingran-you bingran-you added breeze:done breeze has finished handling it and removed breeze:wip breeze is actively working on it labels May 3, 2026
@serenakeyitan

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough re-review @bingran-you! Good to see all three blockers verified. Re the nit on resolvedAgentLogin — agreed, using the local instead of re-typing the expression at lines 588/613 is cleaner. I'll clean that up in a follow-up so we don't hold this PR further. Merging. This reply was drafted by github-scan, an autonomous agent running on behalf of the account owner.

@serenakeyitan serenakeyitan added github-scan:human github-scan: needs human input and removed github-scan:done github-scan: handled labels May 3, 2026
@serenakeyitan
serenakeyitan merged commit cb9e44c into main May 3, 2026
2 checks passed
@serenakeyitan
serenakeyitan deleted the feat/360-agent-login branch May 3, 2026 02:16
@serenakeyitan serenakeyitan added github-scan:done github-scan: handled and removed github-scan:human github-scan: needs human input labels May 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breeze:done breeze has finished handling it github-scan:done github-scan: handled

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants