CXP-383 Support Last Log In w/ usage event feed for the Enterprise connector by JavierCarnelli-ConductorOne · Pull Request #188 · ConductorOne/baton-github

JavierCarnelli-ConductorOne · 2026-09-02T16:24:11Z

Adds an opt-in usage event feed that streams each org's audit-log activity (web/API actions, excluding raw git operations and bot actors) as incremental usage events, letting the platform derive last-activity for members without a per-user API field to sync directly. The config field is intentionally hidden from this connector's CLI/GUI since it only applies to GitHub Enterprise audit-log access; the enterprise connector variant sets it directly on the shared config struct.

This feature is build to be used by baton-github-enterprise which vendors this connector.

Adds an opt-in usage event feed that streams each org's audit-log activity (web/API actions, excluding raw git operations and bot actors) as incremental usage events, letting the platform derive last-activity for members without a per-user API field to sync directly. The config field is intentionally hidden from this connector's CLI/GUI since it only applies to GitHub Enterprise audit-log access; the enterprise connector variant sets it directly on the shared config struct. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

linear-code · 2026-09-02T16:24:13Z

CXP-383

github-actions · 2026-09-02T16:35:10Z

Connector PR Review: CXP-383 Support Last Log In w/ usage event feed for the Enterprise connector

Blocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 145e427ef1fa.
Review mode: incremental since 99f22ebf
View review run

Review Summary

The new commit is a pure rename: usageAppDisplayName and resourceTypeUsageApp.DisplayName go from "GitHub" to "GitHub Activity". The resource type id (usage-app) and resource id (github) are untouched, so this is a display-name-only change and non-breaking (B9); the same constant still feeds both the synced app resource and the usage event's TargetResource, so the two stay consistent. The full PR diff (config field, connector wiring, usage_app.go, usage_event_feed.go, tests) was re-scanned for security and correctness with no new issues found, and the previously fixed newWithGithubApp wiring gap, all-filtered-page pagination bug, error classification, and out-of-bounds cursor guard all remain in place. Two suggestions from earlier passes are still open and carried forward.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

pkg/connector/usage_event_feed.go:160-166 (carried over, still open) — when a response carries no X-Ratelimit-* headers, extractRateLimitData (pkg/connector/helpers.go:184-228) returns a nil error with Remaining: 0 / STATUS_OVERLIMIT; because the tightest-limit comparison only looks at Remaining, that header-less value always wins, so a successful pass can end up carrying a spurious over-limit annotation.
pkg/connector/usage_app.go:48-63 (carried over, still open) — the access entitlement is declared grantable to users but never receives a grant (SkipGrants on the resource type plus Grants returning nil), leaving an entitlement with zero holders; worth confirming the usage uplift can derive the app user from ActorResource alone.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `pkg/connector/usage_event_feed.go`:
- Around line 160-166: `extractRateLimitData` returns a non-error result even when the
  response carries no `X-Ratelimit-Limit` / `X-Ratelimit-Remaining` / `X-Ratelimit-Reset`
  headers, producing `Limit: 0, Remaining: 0, Status: STATUS_OVERLIMIT`. Since the
  tightest-limit selection compares only `rl.GetRemaining()`, that synthetic zero always
  beats a real value and can attach an over-limit annotation to an otherwise healthy pass,
  causing the SDK to back off unnecessarily. Fix by skipping the candidate when the
  response carried no rate-limit headers at all (for example, check that
  `X-Ratelimit-Remaining` is non-empty before considering it), or by having the comparison
  ignore descriptions whose `Limit` is 0.

In `pkg/connector/usage_app.go`:
- Around line 48-63: `Entitlements` declares an `access` assignment entitlement grantable to
  `resourceTypeUser`, but `resourceTypeUsageApp` carries `SkipGrants` and `Grants` returns
  `nil`, so the entitlement will always have zero holders in C1. Either confirm with the
  usage-uplift owners that the entitlement is only needed as a target shape and document
  that in the comment, or drop the entitlement if the uplift keys solely off
  `UsageEvent.ActorResource`.

github-actions

Blocking issues found — see review comments.

newWithGithubApp built its GitHub struct without copying SyncLastActivity, so EventFeeds() always returned nil for App-authenticated connectors regardless of the config value, with no error or log. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

- Track the since-boundary explicitly instead of inferring it from whether an event was emitted, so a page of only filtered-out entries (bots, missing IDs) no longer ends an org's walk early and silently drops later pages of real activity. - Surface the tightest rate limit seen across a call's audit-log requests as an annotation, including on error responses, so the SDK can pace polling instead of hitting 429s. - Fall back to a synthesized event id (org/actor/timestamp/action) when GitHub omits _document_id, avoiding ambiguous dedup on empty ids. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…f them Skip-and-continue is now restricted to permanent per-org conditions (403/404), logged at Warn per this repo's log-level convention. Rate limits, 5xx, and any other error now abort the call instead of silently completing the pass and permanently losing the unfetched activity window on the next poll. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Use nanosecond precision for the synthesized event id and the persisted "since" cursor to avoid same-second id collisions and duplicate re-emitted events. Also check every audit entry's raw timestamp against the boundary, not just ones that pass the bot filter, so an all-filtered page doesn't stall pagination past "since". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

The page budget is a single counter shared across every org processed in a call, not a per-org cap as the comment claimed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Requires review. Bugs fixed

github-actions

No blocking issues found.

github-actions

No blocking issues found.

mateoHernandez123 · 2026-09-03T18:54:20Z

+				return nil, nil, nil, wrapGitHubError(err, resp,
+					fmt.Sprintf("baton-github: failed to fetch audit log for org %s", orgName))
+			case isNotFoundError(resp) || isPermissionError(resp):
+				l.Warn("org lacks audit-log access, skipping it for this pass",


This skip-and-continue 403/404 should be Debug, not Warn.

A missing audit-log permission is an expected per-org skip, not a connector failure — Warn just noise on every pass. Same shape as the rest of this connector: Debug for skippable 4xx, Error only for 5xx / bugs.

Hey Tute! Thanks for the review.
In this case we want to keep it as Warn since it's an error condition, not an expected/normal data state.
You can see it in patterns-error-handling.md as Rule 4

GitHub orgs aren't a GitHub App install, so C1's usage uplift never picks up org-targeted usage events. Mimic baton-dropbox's pattern: sync a static TRAIT_APP resource with a real entitlement, gated behind sync-last-activity, and point usageEventFeed's UsageEvents at it instead of the org. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

github-actions

No blocking issues found.

github-actions

No blocking issues found.

mateoHernandez123 · 2026-09-10T17:01:26Z

 		resourceSyncers = append(resourceSyncers, APITokenBuilder(gh.client, gh.orgCache))
 	}

+	if gh.syncLastActivity {


Non-blocking suggestion — fine as a follow-up, not a condition for this PR.

usageAppBuilder is registered only while syncLastActivity is true, so flipping the enterprise toggle from true to false drops the whole usage-app resource type out of ResourceSyncers(), and C1 reads a previously synced type disappearing as a deletion. Registering the builder unconditionally and gating only EventFeeds() would keep the resource catalog stable across a config change.

Consumer config: https://github.com/ConductorOne/baton-github-enterprise/blob/52c1fbed32d34b623bb5aa80afb655b01714b9ee/pkg/config/config.go#L80-L113

Deliberate tradeoff, not planning to change it: this entitlement never has grants (Grants() returns nil, resource type carries SkipGrants), so a toggle-triggered "deletion" is just catalog churn, not an access change. Registering it unconditionally would instead give every plain baton-github customer (sync-last-activity is hidden/CLI-only there) a permanent, unexplained "GitHub / access" resource with no function. Gating avoids that at the cost of occasional harmless churn for the few admins who actually flip the enterprise toggle.

mateoHernandez123 · 2026-09-10T17:01:26Z

 }

+func (gh *GitHub) EventFeeds(_ context.Context) []connectorbuilder.EventFeed {
+	if !gh.syncLastActivity {


Non-blocking suggestion — worth a follow-up once the enterprise side lands.

The feat/cxp-383 branch wires SyncLastActivity, but DefaultCapabilitiesBuilder has no EventFeeds() and its generated capabilities contain neither usage-app nor CAPABILITY_EVENT_FEEDS; both new constructors here are private, so downstream cannot reuse them. Exporting or sharing the feed/resource capability path (and regenerating the enterprise metadata) would let the consumer advertise the feed.

Consumer builder: https://github.com/ConductorOne/baton-github-enterprise/blob/52c1fbed32d34b623bb5aa80afb655b01714b9ee/pkg/connector/connector.go#L31-L62

github-actions

No blocking issues found.

JavierCarnelli-ConductorOne self-assigned this Sep 2, 2026