feat(observability): authenticate hosted Factory reporter - #289
feat(observability): authenticate hosted Factory reporter#289khaliqgant wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughHosted Cloud reporting now supports rotating access tokens and injected transports. API URL resolution and token timeout handling are centralized. Tests cover successful delivery, outbox draining, and fail-open behavior on telemetry HTTP 503 responses. ChangesHosted Cloud reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change authenticates hosted telemetry while preserving local reporting and fail-open command behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant FleetCli as Fleet CLI
participant TokenEndpoint as Hosted access-token endpoint
participant Telemetry as Cloud telemetry
participant Outbox as File cloud event outbox
FleetCli->>TokenEndpoint: Request rotating access token
TokenEndpoint-->>FleetCli: Return access token
FleetCli->>Telemetry: Submit lifecycle events with bearer token
Telemetry-->>FleetCli: Return delivery result
FleetCli->>Outbox: Drain delivered events
Telemetry-->>FleetCli: Return HTTP 503
FleetCli->>Outbox: Preserve pending events
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2dd51921f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| getAccessToken = createHostedCloudAccessTokenProvider({ | ||
| url: runtimeEnv[FACTORY_CLOUD_ACCESS_TOKEN_URL_ENV]?.trim() ?? '', | ||
| fetchImpl: input.deps.cloudAccessTokenFetch ?? fetch, | ||
| }) |
There was a problem hiding this comment.
Bound hosted-token fetches to reporter shutdown
When the private token endpoint hangs while an automatic flush is already running, close({ deadlineMs: 2_000 }) only stops awaiting the reporter operation; it does not cancel this hosted-token provider. The provider retains a referenced 10-second abort timer, and the in-flight reporter can perform three attempts, so main() may set the exit code while Node remains alive for roughly 30 seconds. Thread reporter cancellation into the token request, or otherwise ensure the request cannot keep the process alive after the shutdown deadline.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/cli/fleet.ts">
<violation number="1" location="src/cli/fleet.ts:1625">
P2: Propagate reporter cancellation into `createHostedCloudAccessTokenProvider`; otherwise a hung hosted-token fetch continues past `close({ deadlineMs: 2_000 })` and can keep the CLI process alive until its timeout and retries finish.</violation>
<violation number="2" location="src/cli/fleet.ts:1626">
P2: Hosted mode is selected by the presence of FACTORY_CLOUD_ACCESS_TOKEN_URL, not by a non-empty value. If the variable is set to an empty or whitespace-only string, the code enters the hosted branch and createHostedCloudAccessTokenProvider throws synchronously on `new URL('')`, so hosted telemetry is bypassed/disabled (or the command errors) instead of falling back to the local-session path. Gate the hosted branch on a trimmed non-empty value and pass that value to the provider.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| let cloudFetch: typeof fetch | undefined | ||
| if (hasHostedAccessTokenConfig) { | ||
| apiUrl = resolveHostedCloudApiUrl(runtimeEnv) | ||
| getAccessToken = createHostedCloudAccessTokenProvider({ |
There was a problem hiding this comment.
P2: Propagate reporter cancellation into createHostedCloudAccessTokenProvider; otherwise a hung hosted-token fetch continues past close({ deadlineMs: 2_000 }) and can keep the CLI process alive until its timeout and retries finish.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/fleet.ts, line 1625:
<comment>Propagate reporter cancellation into `createHostedCloudAccessTokenProvider`; otherwise a hung hosted-token fetch continues past `close({ deadlineMs: 2_000 })` and can keep the CLI process alive until its timeout and retries finish.</comment>
<file context>
@@ -1594,15 +1613,28 @@ async function buildFactoryCloudReporter(input: {
+ let cloudFetch: typeof fetch | undefined
+ if (hasHostedAccessTokenConfig) {
+ apiUrl = resolveHostedCloudApiUrl(runtimeEnv)
+ getAccessToken = createHostedCloudAccessTokenProvider({
+ url: runtimeEnv[FACTORY_CLOUD_ACCESS_TOKEN_URL_ENV]?.trim() ?? '',
+ fetchImpl: input.deps.cloudAccessTokenFetch ?? fetch,
</file context>
| if (hasHostedAccessTokenConfig) { | ||
| apiUrl = resolveHostedCloudApiUrl(runtimeEnv) | ||
| getAccessToken = createHostedCloudAccessTokenProvider({ | ||
| url: runtimeEnv[FACTORY_CLOUD_ACCESS_TOKEN_URL_ENV]?.trim() ?? '', |
There was a problem hiding this comment.
P2: Hosted mode is selected by the presence of FACTORY_CLOUD_ACCESS_TOKEN_URL, not by a non-empty value. If the variable is set to an empty or whitespace-only string, the code enters the hosted branch and createHostedCloudAccessTokenProvider throws synchronously on new URL(''), so hosted telemetry is bypassed/disabled (or the command errors) instead of falling back to the local-session path. Gate the hosted branch on a trimmed non-empty value and pass that value to the provider.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/fleet.ts, line 1626:
<comment>Hosted mode is selected by the presence of FACTORY_CLOUD_ACCESS_TOKEN_URL, not by a non-empty value. If the variable is set to an empty or whitespace-only string, the code enters the hosted branch and createHostedCloudAccessTokenProvider throws synchronously on `new URL('')`, so hosted telemetry is bypassed/disabled (or the command errors) instead of falling back to the local-session path. Gate the hosted branch on a trimmed non-empty value and pass that value to the provider.</comment>
<file context>
@@ -1594,15 +1613,28 @@ async function buildFactoryCloudReporter(input: {
+ if (hasHostedAccessTokenConfig) {
+ apiUrl = resolveHostedCloudApiUrl(runtimeEnv)
+ getAccessToken = createHostedCloudAccessTokenProvider({
+ url: runtimeEnv[FACTORY_CLOUD_ACCESS_TOKEN_URL_ENV]?.trim() ?? '',
+ fetchImpl: input.deps.cloudAccessTokenFetch ?? fetch,
+ })
</file context>
Summary
FACTORY_CLOUD_ACCESS_TOKEN_URLrotating-token provider for hosted telemetry deliveryHosted contract
The reporter and Relayfile mount now share the existing hosted Cloud API resolver and the same validated access-token provider. The provider retains its URL/protocol, timeout, HTTP response, response-body, and
relay_pa_token-class checks.This introduces no new secret, variable, or deployment gate.
CLOUD_API_URLremains optional with its existing default, andFACTORY_CLOUD_ACCESS_TOKEN_URLremains the private rotating-token endpoint already supplied by the hosted runtime.Cloud must land and deploy AgentWorkforce/cloud#3083 before this path is rolled out. That PR adds the narrow ingest authorization and verified Relay-workspace binding.
Verification
npx vitest run src/cli/fleet.test.ts src/mount/relayfile-cloud-mount-client.test.ts— 204 passednpx vitest run --maxWorkers=4— 1,757 passed, 1 skippednpm run buildnpx prettier --check src/cli/fleet.ts src/cli/fleet.test.ts src/mount/relayfile-cloud-mount-client.tsgit diff --checkThe hosted success test exercises real reporter serialization, bearer auth, cloud instance identity, lifecycle batches, and a drained outbox without invoking local login. The 503 test proves the Factory command still succeeds while telemetry remains pending for retry.
Veto MCP tools requested by repository instructions were not present in this session, so no Veto result is claimed. Manual diff and secret reviews were completed.
Rollout and evidence
No deployment or merge was performed here. Far-end delivery remains UNVERIFIED until step 4 is observed.
Summary by cubic
Authenticate the hosted Factory telemetry reporter using the existing private rotating-token endpoint. Previously the reporter required a local Cloud session; now, when hosted, it fetches a short‑lived
relay_pa_token and sends batches directly, preserving fail‑open behavior.FACTORY_CLOUD_ACCESS_TOKEN_URLis present; it resolvesCLOUD_API_URLviaresolveHostedCloudApiUrl, skipscloudSessionProvider, and validates URL/protocol and token class.ensureCloudSessionand the session’sapiUrland access token.createHostedCloudAccessTokenProvider(with a default timeout), add test-only DI hookscloudAccessTokenFetchandcloudReporterFetch; no new secrets or envs required, andCLOUD_API_URLremains optional.FACTORY_CLOUD_ACCESS_TOKEN_URL; non-hosted users are unaffected.Written for commit e2dd519. Summary will update on new commits.