You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Always send OBS exports to /observabilityService, including batch and per-request exports. Retain useS2SEndpoint for compatibility but ignore it, even when false; never fall back to /observability.
Add an app-only resolver overload for AgenticTokenCache.RefreshObservabilityToken. The legacy user-authorization overload now fails explicitly instead of acquiring a delegated OBS token.
Surface token acquisition failures, clear stale expiry metadata when refreshing opaque tokens, and document the migration. Workload MCP/Graph/OBO authentication is unchanged.
Compatibility
This changes OBS routing and the hosting cache's authentication contract. Callers must supply an app-only OBS token for the exporting agent and tenant; a delegated scp token cannot authenticate the S2S route. Endpoint selection does not mint or convert tokens.
Validation
93 targeted tests passed across the exporter, builder/configuration, hosting cache, and output middleware.
Runtime, observability, and hosting packages built in CJS and ESM; changed TypeScript passed ESLint.
Covered omitted/false/true legacy flags, domain overrides, per-request routing, no OBO fallback on 401/403/404, app-only cache callbacks, error propagation, and expiry.
Authenticated live AI Teammate/OBO ingestion remains unverified: a working provisioned agent identity and authentic OBO test assertion are still required. No live ingestion success is claimed.
Always route OBS to observabilityService, retain the legacy endpoint option as ignored compatibility state, and replace delegated hosting-cache exchange with an explicit app-only resolver.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
There are a couple of actionable review findings (type-only imports to avoid runtime dependencies and a brittle/slow cache-capacity test) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the observability (OBS) export pathing and hosting token cache contract to enforce S2S-only export routing and require app-only token acquisition, aligning exporter behavior, tests, and documentation with the new authentication model.
Changes:
Route all OBS exports (batch + per-request) to /observabilityService and deprecate/ignore useS2SEndpoint (even when false).
Change hosting AgenticTokenCache.RefreshObservabilityToken to require an app-only resolver; legacy TurnContext/Authorization overload now throws and token acquisition failures propagate.
Update docs/changelog and adjust tests to cover the new endpoint and token acquisition/expiry behaviors.
The reason will be displayed to describe this comment to others. Learn more.
Full first-pass panel review
Verdict: Needs work (high risk). The S2S-only route is a security-sensitive public authentication-contract change. Batch export and the hosting cache now have an app-only resolver path, but per-request export still ignores that resolver and forwards the existing OTel-context token to the new S2S endpoint. That leaves a supported mode unable to satisfy the contract and can drop all per-request telemetry with 401/403 responses.
Blocking
packages/agents-a365-observability/src/tracing/exporter/Agent365Exporter.ts:192 - Per-request export is routed to /observabilityService, but the token branch at lines 210-212 still always uses getExportToken(). ObservabilityBuilder.createPerRequestProcessor() also does not propagate a configured tokenResolver, and the changed test explicitly proves an arbitrary context token is forwarded. Existing AI Teammate/OBO callers therefore keep sending delegated tokens that the PR says S2S rejects. Wire an app-only resolver into per-request export (or introduce a distinctly contracted app-only context), update the public token-context migration guidance, and add a regression that distinguishes delegated from app-only acquisition.
Existing unresolved review items (not duplicated)
Copilot: use type-only imports in AgenticTokenCache.ts.
Copilot: avoid hard-coding and iterating through 10,001 cache entries in the eviction test.
Trade-off
Removing the delegated/OBO fallback is the correct security direction; the fix should preserve that invariant rather than restore the old route. The missing piece is an app-only token source for every export mode.
Persona roll-up
Security: Blocking token-source/trust-boundary mismatch in per-request mode; no secret exposure or authorization fallback added elsewhere.
Privacy: No new collection, retention, or tenant-mixing path; the loss-of-user-attribution caveat is documented.
Performance: Retries and cache lifetime remain bounded; the expensive eviction test is already covered by an existing thread.
Customer service: The changelog describes the break, but per-request consumers lack a working migration path.
Business / COGS: No material storage, egress, cardinality, or provisioning increase.
Senior engineer: The changed test verifies routing but not the new authentication invariant, allowing a production telemetry outage to pass.
Architect: Batch/cache and per-request modes now implement different credential contracts behind one public exporter API.
Feedback ledger
No repository-specific ledger exists yet. The two existing Copilot findings were suppressed from new inline comments to avoid duplication.
Approval gate
Not approved: one blocking finding remains, two prior review threads are unresolved, and the branch is behind main. All exact-head CI checks currently pass, but green CI does not exercise the delegated-token counterfactual described above.
Use the configured OBS resolver for batch and per-request export without ambient-token or OBO-route fallback. Fail missing-token exports explicitly, add delegated-context counterfactuals and builder integration coverage, declare the tooling axios dependency, and update migration guidance and cache tests.
Review response amendments:
- Startup error now names the fix ("Per-request export now requires withTokenResolver(...)") and points at AgenticTokenCache for caching.
- README and CHANGELOG document that resolvers must cache; the exporter invokes the resolver on every batch and per identity group.
- Defensive resolver guard in exportGroup now comments that it only catches post-construction mutation; the constructor is the primary check.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
Bring in main's dependency security overrides. Align the new tooling axios dependency with main's axios override (^1.16.0, resolved 1.20.0) and record it with the override specifier in pnpm-lock.yaml, matching how main records hono.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
Per-request export now uses the configured app-only tokenResolver, the same as batch export. Agent365Exporter no longer reads getExportToken(), so a token in runWithExportToken is never sent to OBS.
An enabled exporter without a resolver fails at configuration, and the error names the fix (withTokenResolver(...)). Empty tokens or failed acquisition fail the export without sending a request. There is no OBO fallback.
AgenticTokenCache uses type-only imports; the eviction test derives the cache capacity.
@microsoft/agents-a365-tooling now declares axios, which it imports directly. After merging main, the catalog range matches main's axios security override (^1.16.0).
The README, CHANGELOG and hosting design doc describe the per-request migration and note that resolvers should cache tokens, because the exporter calls them for each export batch.
Breaking change: per-request users who relied on runWithExportToken must configure withTokenResolver(...). See the README section Migrating per-request authentication.
Validation
Merged tree: all packages build (CJS and ESM), lint is clean, and the full unit suite passes (65 suites, 1,314 tests).
New regressions: a delegated JWT in the request context is never exported, while the resolver's roleless app token is. They run through the real builder, per-request processor and exporter, and also cover concurrent identities, resolver precedence, and missing or failed acquisition.
Live: an earlier head of this PR exported real agent telemetry through the S2S OTLP route using a roleless app-only token (HTTP 200, downstream processing confirmed). The per-request changes in this update are covered by offline tests only.
The reason will be displayed to describe this comment to others. Learn more.
Delta re-review (93f1934 -> 65ffbcc)
Verdict: Needs work (high-risk authentication contract). The original blocking per-request credential defect is fixed at the new head, and I found no additional issues beyond the two existing Copilot threads about the public exporterOptions path.
Author-claimed fixes
Accepted - per-request app-only resolver:ObservabilityBuilder.createPerRequestProcessor() now uses the same createExporterOptions() path as batch export, and Agent365Exporter.exportGroup() obtains credentials only from options.tokenResolver. The new real builder/processor/exporter tests distinguish a delegated context token from the resolver token and cover concurrent identities and resolver precedence.
Accepted - fail closed without a resolver: the exporter constructor rejects a missing resolver; empty results and acquisition failures fail before fetch; 401/403/404 remain on /observabilityService without an OBO fallback. Exact-head tests cover each branch.
Accepted - hosting cache fixes:AgenticTokenCache now uses type-only hosting imports, clears stale expiry/acquisition metadata after failures or opaque-token refreshes, and the eviction test derives _maxCacheSize while reusing one resolver.
Accepted - tooling dependency:@microsoft/agents-a365-tooling now declares its direct axios dependency through the workspace catalog and has a manifest regression test.
Partially accepted - migration documentation: the README, changelog, and hosting design now explain the breaking per-request migration and resolver caching. However, the README also presents exporterOptions.tokenResolver as equivalent without qualifying that it only works through ObservabilityBuilder.withExporterOptions(...). ObservabilityManager.start(options) accepts BuilderOptions but does not forward options.exporterOptions; the exact-head regression tests exercise the builder directly, not this public convenience API.
Accepted - validation evidence: all exact-head GitHub checks are complete and green, including Node.js 18/20, CodeQL, and JavaScript/TypeScript analysis. The reported live S2S success was on an earlier head; the PR correctly limits the new per-request claim to offline coverage.
Existing approval blockers
The source-valid ObservabilityManager.start(options) / exporterOptions.tokenResolver gap remains represented by the README thread and the design-doc thread. These are one underlying public API/migration issue and are not duplicated here. Either forward exporterOptions in ObservabilityManager.start with a regression test, or scope the documentation to the working withExporterOptions({ tokenResolver }) builder path.
The prior panel blocker was source-verified as fixed in 78deb5d and its thread has been resolved. The type-import and cache-capacity findings are also source-verified and resolved. No new inline findings were added.
Persona roll-up
Security: app-only credential selection is now consistent across batch and per-request modes; no delegated fallback or new secret exposure found.
Privacy: no new data collection, retention, residency, or tenant-mixing issue found.
Performance / COGS: resolver calls are explicitly documented as cache-required; no new unbounded runtime work or material cost issue found.
Customer service: the remaining public-options ambiguity can send a documented migration path to a startup exception.
Senior engineer / Architect: implementation and focused regressions close the original mode-skew defect; the public convenience API is still inconsistent with its accepted options type.
Approval gate: not approved because two source-valid review threads remain unresolved. All substantive CI checks are green; merge remains subject to branch protection.
ObservabilityManager.start(options) accepted BuilderOptions.exporterOptions
but never passed it to the builder, so the documented
`exporterOptions.tokenResolver` migration path failed at startup with the
now-required app-only resolver. Forward it through withExporterOptions; a
top-level tokenResolver still takes precedence.
Regression tests exercise the public start() path through the real
processor and exporter and fail without the fix.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
Regenerate lockfile to preserve axios catalog mapping
pnpm-lock.yaml:500
The package manifest uses axios: catalog:, but this importer records the dependency as the literal ^1.16.0, and the lockfile has no corresponding axios entry under catalogs.default. Regenerate pnpm-lock.yaml with pnpm so the catalog mapping and importer stay consistent; otherwise frozen installs can report the lockfile as out of date or resolve this dependency differently.
State in the README, design guide, and changelog that
ObservabilityManager.start(options) forwards exporterOptions (including
exporterOptions.tokenResolver), and that tokenResolver/withTokenResolver
takes precedence.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
6464933: ObservabilityManager.start(options) now forwards options.exporterOptions, so start({ exporterOptions: { tokenResolver } }) works. A top-level tokenResolver still takes precedence. The regression tests go through the public start() path with the real processor and exporter, and fail without the fix.
dd4a8be (docs only): the README, design guide, and CHANGELOG now say that start(options) forwards exporterOptions.
Validation on 6464933: pnpm build, pnpm lint, and all 1316 unit tests pass locally.
Jason-R-Lien, both threads are resolved; ready for another look.
The reason will be displayed to describe this comment to others. Learn more.
Delta re-review (65ffbcc -> dd4a8be)
Verdict: re-review complete with no new source-valid findings. The requested ObservabilityManager.start(options) fix is present and covered, but I am leaving a COMMENT rather than approving because two later Copilot threads remain unresolved. Both late findings are source-rebutted below; thread resolution is the only remaining approval-gate condition.
Author-claimed fixes
Accepted - exporterOptions forwarding:ObservabilityManager.start() now passes options.exporterOptions to withExporterOptions(). ObservabilityBuilder.createExporterOptions() merges those values and then applies the top-level tokenResolver, preserving the documented precedence.
Accepted - public-path regression coverage:observabilityManager-exporter-options.test.ts exercises the real public start() path through the processor/exporter, proves exporterOptions.tokenResolver exports successfully, and proves a top-level resolver wins.
Accepted - documentation: README, design guide, and CHANGELOG now accurately state that start(options) forwards exporterOptions.
Accepted - validation: every exact-head check is complete and green, including Node.js 18/20, CodeQL, and JavaScript/TypeScript analysis.
Late review reconciliation
Rejected - lockfile is unsynchronized:pnpm-lock.yaml records the workspace catalog's Axios range as ^1.16.0 because the same package is pinned by the root override; the existing overridden hono catalog entry has the same resolved-specifier shape. Exact-head CI's pnpm i succeeds on both Node versions before build/test/pack.
Rejected - Jest 30 typing does not compile: this test uses the repository's global jest type from @types/jest, and the exact file compiles and passes under the configured ts-jest diagnostics on both Node 18 and 20. The one-generic @jest/globals signature cited by the thread is not the binding used here.
Approval gate
No blocking or should-fix issue remains in the code, docs, tests, security/privacy boundary, performance path, or package contract. Approval is withheld solely because the lockfile thread and the Jest typing thread are still unresolved; the gate requires every live review thread to be resolved even when its technical claim is rebutted. Merge remains subject to branch protection.
Rick Brighenti (@rbrighenti), both items are addressed in ceea486 (details inline): the release is now 2.0.0 with its own CHANGELOG section, and the removed overload no longer throws at runtime.
Also from your review of microsoft/Agent365-Samples#339: the hosting design doc now states the same roleless app-only contract as the samples' providers. That is idtyp=app, non-empty roles, or absent idtyp with a non-empty oid equal to sub; any scp is rejected.
Follow-up I'm tracking separately: the exporter's 403 log is still generic. It could point to instance registration.
The reason will be displayed to describe this comment to others. Learn more.
Delta re-review (dd4a8be -> ceea486)
Verdict: needs work. The requested release-version and removed-overload fixes are accepted from exact-head source and tests. I found one new should-fix in the changed authentication guidance; the two author-addressed review threads also remain unresolved, and the prior CHANGES_REQUESTED review is still active.
Author-claimed fixes
Accepted - major version:version.json now uses 2.0.0-preview.{height}, matching HOW_TO_RELEASE.md's major-version rule for incompatible API changes.
Accepted - release notes: the OBS changes are under ## [2.0.0] - Unreleased; the prior block is labeled 1.0.0, and comparison of v1.0.0 to current main confirms no intervening mainline CHANGELOG.md changes.
Accepted - TypeScript migration failure: the public method now has only the resolver signature, and the @ts-expect-error regression fails if the removed TurnContext/Authorization overload becomes typeable again.
Accepted - JavaScript compatibility behavior: non-function legacy calls log once per cache instance and return without token exchange or cache mutation; the regression invokes the untyped legacy shape twice and verifies no throw, no exchange, no token, and one error.
Accepted - acquisition-failure guidance: the source JSDoc, observability README, hosting design, and changelog consistently state that resolver acquisition failures throw and must be contained on a request path.
Rejected in part - sample-contract parity: the new design sentence does not preserve the samples' requirement that roles is an app-only signal only when idtyp is absent. The inline finding has the exact wording correction.
Not re-raised - generic 403 guidance: the author explicitly tracks this separately; this delta makes no claimed fix there, and it is not a new approval blocker from this pass.
Approval gate
One new should-fix remains, both late reviewer threads are unresolved, and the earlier CHANGES_REQUESTED review has not been cleared. Node.js 18/20 checks are still in progress at review time; all completed substantive checks are successful. Merge remains subject to branch protection.
Persona roll-up
Security / Customer Service / Architect found the auth-contract wording mismatch. Senior Engineer found the requested runtime and typing behavior covered by mutation-sensitive tests. Privacy, Performance, and Business/COGS found no delta-specific concerns.
Non-empty roles and oid == sub count as app-only only when idtyp is absent,
and any other idtyp value is rejected.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
Agent365Exporter imports ExportResultCode from @opentelemetry/core, but the
package never declared it, so installs where npm did not hoist another copy
failed with MODULE_NOT_FOUND when loading the exporter (found while moving
the Agent365-Samples Node samples to 1.0.0). It now uses the workspace
catalog version like the LangChain extension.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
acdb9b5: addresses Jason's should-fix. The hosting design doc now states the exact app-only predicate the samples enforce: idtyp=app, or, when idtyp is absent, a non-empty roles array or a non-empty oid == sub. Any other idtyp is rejected.
14fad0c:@microsoft/agents-a365-observability now declares @opentelemetry/core (the catalog's ^2.6.0).
Agent365Exporter imports ExportResultCode from it, but the package never declared it.
Moving the Agent365-Samples Node samples to 1.0.0, which has the same gap, surfaced MODULE_NOT_FOUND whenever npm didn't hoist another copy.
The lockfile change is only the 3-line importer entry (pnpm install --frozen-lockfile passes), and the packed package.json lists @opentelemetry/core: ^2.6.0.
Build and lint are clean, and all 1317 tests pass. There's a Fixed entry under 2.0.0 in the CHANGELOG.
I resolved Rick's two earlier threads, which were addressed in ceea486 and approved. Rick, these pushes probably dismissed your approval; sorry for the churn.
The reason will be displayed to describe this comment to others. Learn more.
Delta re-review (ceea486 -> 14fad0c)
Verdict: approved. No new blocking or should-fix findings remain.
Author-claimed fixes
Accepted - app-only predicate:acdb9b5 now states the same validation contract as the referenced TypeScript, C#, and Python samples: accept idtyp=app; only when idtyp is absent may non-empty roles or non-empty matching oid == sub establish app-only identity; reject every other idtyp and any delegated scp. I resolved the agent-owned thread after verifying the exact-head text.
Accepted - direct OpenTelemetry dependency:Agent365Exporter.ts imports runtime values ExportResult and ExportResultCode from @opentelemetry/core; 14fad0c now declares that package in the observability package's runtime dependencies, uses the existing ^2.6.0 workspace catalog entry, and records the resolved importer in pnpm-lock.yaml. This removes the undeclared-hoist dependency described by the author.
Accepted - prior reviewer fixes remain intact: the 2.0.0 version/release-note correction and the compile-time removal plus non-throwing JavaScript compatibility path for the old hosting overload were source-verified in the preceding review. Rick's corresponding threads are now resolved, and this delta does not regress either fix.
Not re-raised - generic 403 guidance: the author is tracking this separately; the current delta does not claim to fix it, and it is not an approval blocker from this pass.
Approval gate
All 10 review threads are resolved. The latest Copilot exact-head review reports no findings. Every exact-head check is complete and successful: Node.js 18/20, CodeQL, JavaScript/TypeScript analysis, changed-SDK detection, CLA, and Copilot review. Merge remains subject to branch protection.
Persona roll-up
Security, Customer Service, and Architect accept the corrected authentication-boundary guidance. Senior Engineer accepts the direct runtime dependency and synchronized lockfile. Privacy, Performance, and Business/COGS found no delta-specific concerns.
Both were validated live against the production S2S route. An app-only token with no roles (idtyp=app, no scp) got 200 on /observabilityService/tenants/{t}/otlp/agents/{a}/traces with default options, and again with the deprecated S2S flag set to false. An empty token fails without sending a request. The new hosting cache made one token acquisition across two exports, using the /.default scope.
The reason will be displayed to describe this comment to others. Learn more.
Same-head re-review
Verdict remains approved. The new author comment adds cross-SDK validation and coordinated-release evidence; it does not change this PR's code, requirements, or approval gate, so I am not duplicating the existing exact-head approval.
Author claims reconciled
Accepted - .NET and Python ports exist with matching semantics:microsoft/Agent365-dotnet#265 and microsoft/Agent365-python#275 both route OBS exports only through /observabilityService, use app-only resolver-backed tokens, ignore the deprecated S2S flag even when false, reject empty tokens before sending, use the /.default scope, and add cache-reuse coverage.
Accepted as operational evidence - live S2S validation: both companion PR descriptions record production-route HTTP 200 results for default and deprecated-flag-false configurations, no request for an empty token, and one resolver acquisition across two cached exports. This is author-provided runtime evidence, not independently reproduced in this Node re-review.
Accepted - coordinated release constraint: both companion PRs explicitly say these SDK changes ship together with the related Node, CLI, samples, and skills changes, with the CLI merged last. The .NET companion currently has an open high-severity cache-concurrency review finding, so release coordination is not yet complete; that is a release/merge-order constraint, not a defect in this unchanged Node head.
Approval gate
All 10 Node review threads remain resolved, the aggregate decision is approved, and every exact-head substantive check is complete and successful (Node.js 18/20, CodeQL, JavaScript/TypeScript analysis, changed-SDK detection, CLA, and Copilot review). No new blocking or should-fix finding was introduced. Merge and coordinated shipment remain subject to repository and release-owner controls.
… default (#501)
* Add --skip-observability-permissions to setup all
Blueprint agents that export telemetry through the app-only S2S endpoint
(microsoft/Agent365-nodejs#290, microsoft/Agent365-Samples#339) are
authorized by their agent registration, so the Observability API OtelWrite
permission, and the admin consent it needs, is unnecessary for them.
- New opt-in `setup all --skip-observability-permissions` omits Observability
API from the permission specs (inheritable permissions, app role grants,
batch consent) and from the per-resource and combined admin consent URLs.
Defaults are unchanged: the published SDKs still export to the non-S2S
endpoint by default.
- The flag fails fast for AI Teammate agents and with authMode s2s/both,
since OtelWrite is the only app role those modes grant. A contradicting
--authmode flag is rejected before bootstrap signs in.
- With the flag, a failed agent registration is an error (exit 1), because
registration is then the agent's only Observability authorization.
- Fix: `setup all --agent-registration-only` exited 0 when registration failed.
- Dry run plan, setup summary, CHANGELOG, and docs updated.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Make skipping Observability permissions the default for blueprint agents
Per 3P Dev Scale scrum feedback, the no-consent flow becomes the main
path instead of an opt-in flag.
- Remove --skip-observability-permissions. Blueprint agents in the
default (obo) auth mode no longer request Observability API
permissions; registered agents export telemetry with an app-only
token over the S2S endpoint.
- authMode s2s/both keep requesting OtelWrite, the only app role those
modes grant; `both` also covers agents whose SDK still exports
through the delegated (OBO) route.
- AI Teammate setup is unchanged until instance creation can be
validated end to end.
- Registration failure stays an error on the default path.
- Tests: the default plan omits Observability; s2s/both (flag or
config) keep it; AI Teammate keeps it. Mutation-checked.
Validated live: a roleless app-only token for a registered agent
identity exports 200 on S2S; an unregistered identity gets 403
insufficient_scope.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Stop requesting Observability permissions in every blueprint auth mode
The S2S endpoint authorizes registered agents without OtelWrite whatever
the auth mode, so s2s/both no longer request it either. They still grant
any other app-role specs (e.g. Defender once #485 lands). Agents whose SDK
still exports through the delegated route grant OtelWrite manually, as the
CHANGELOG upgrade note describes. AI Teammate setup is unchanged.
Tests encode the changed requirement for s2s/both (flag or config) and are
mutation-checked.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Address #501 review: registration and consent edge cases
- When registration is required (--agent-registration-only, or Observability
permissions not requested), an inconclusive registration check now fails
setup instead of passing. The stored ID is kept and no duplicate
registration is created. The optional path still retains the stored ID.
- When Observability is not included, drop an Observability consent entry
saved by an earlier run so the admin is not asked for it.
- Keep Observability for an AI Teammate config retained for a dry run (skip
only for an effective blueprint selection).
- Scope the guided-setup OtelWrite grant steps to AI Teammates and SDKs that
still export through the delegated route; fix two stale doc comments.
Regression tests cover each case and are mutation-checked.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Condense #501 changelog entries and refresh stale test wording
Make the upgrade note one consumer-facing sentence, and update the Fixed
entry: setup exits 1 when registration fails or cannot be verified for
blueprint agents as well as with --agent-registration-only. Replace
"without the flag" in a registration test, since the flag was removed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Scope the Observability upgrade note to delegated-route agents
The upgrade note opened by saying every existing agent needs the
Observability permissions, which contradicted the S2S exception. Scope
the heading and requirement to agents that export through the delegated
(OBO) route.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Address #501 review: s2s/both summary and failed app-role hand-off
- Setup summary: blueprint agents no longer request any app role (OtelWrite
was the only one), so an s2s or both run has no S2S grant. The Blueprint
Permission Grants row now says so instead of reporting a delegated grant, or
a PENDING with no action item for non-admin s2s runs: "not required (no S2S
app roles to grant)" for s2s, and the delegated status plus "no S2S app roles
to grant" for both.
- The S2S PowerShell hand-off lists the app roles that were actually not
assigned, recorded per blueprint and agent identity, instead of hardcoding
Observability OtelWrite. AI Teammates still get the OtelWrite step because
they still request it.
- A stale Observability consent URL is also cleared on admin runs, and only
the URL is cleared: ConsentGranted and the inheritable-permission state are
kept, since re-running setup does not revoke.
- Document why registration is always required in real runs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Align #501 docs with the s2s/both summary
- CHANGELOG upgrade note: the setup summary prints the OtelWrite PowerShell
steps only for AI Teammates now, so point blueprint agents on the delegated
route to Option A.
- Setup README: s2s and both have no app role to assign for blueprint agents,
and the summary reports the S2S grant as not required.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* List a hand-off per failed S2S target in the setup summary
When the blueprint and agent-identity app-role grants both fail in one run,
the summary printed only the agent identity's roles and dropped the
blueprint's. It now prints one hand-off per failed target, each listing that
target's pending roles and principal. A single failed target prints as
before.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Qualify the s2s/both README note and de-duplicate pending roles
- README: s2s and both have nothing to assign for blueprint agents only when
no other permission adds an app role.
- Setup summary: a role recorded twice for the same target is listed once.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Address observability S2S review feedback
Align guided setup docs with the app-only S2S telemetry model, fail setup when a missing blueprint secret blocks required registration, and update dry-run/help/summary remediation for skipped OtelWrite.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Clarify delegated-route upgrade note
Point new blueprint agents that still use the delegated Observability route to the custom permissions command that stamps inheritable permissions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Keep the setup admin removal note and fix the missing-secret retry advice
- CHANGELOG upgrade note: keep stating that `a365 setup admin` was removed in
this release, next to the new `setup permissions custom` route.
- Missing blueprint secret: the error now says to re-run `a365 setup blueprint`
and then `a365 setup all`. `--agent-registration-only` skips identity creation,
so it can't recover a run that never created the agent identity. The test pins
this.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Point the Observability opt-back-in command at the cloud's app ID
After merging #478, sovereign clouds use their own Observability app IDs, so
the README's opt-back-in command no longer hard-codes the commercial ID.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Address observability opt-back-in review
Separate default Observability omission from effective requested permissions when custom Observability permissions are configured, and update dry-run/help/docs for cloud-aware S2S guidance.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Tighten observability opt-back-in handling
Require custom Observability opt-back-in to target the configured cloud and OtelWrite, track identity and registration failure severity explicitly, and clarify delegated-route documentation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
* Record the auth mode before the agent identity step
EffectiveAuthMode and NoS2SAppRolesToGrant were set only after an agent
identity existed, so when identity creation failed an s2s/both run with no
app roles to grant could be summarized as a pending or delegated grant
instead of "no S2S app roles to grant".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
---------
Co-authored-by: Krishnadheeraj <12496535+DheerajPannala@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cbf5f6b-cc40-4b7e-a591-65848db73a12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Jason-R-Lien
left a comment
Summary
/observabilityService, including batch and per-request exports. RetainuseS2SEndpointfor compatibility but ignore it, even when false; never fall back to/observability.AgenticTokenCache.RefreshObservabilityToken. The legacy user-authorization overload now fails explicitly instead of acquiring a delegated OBS token.Compatibility
This changes OBS routing and the hosting cache's authentication contract. Callers must supply an app-only OBS token for the exporting agent and tenant; a delegated
scptoken cannot authenticate the S2S route. Endpoint selection does not mint or convert tokens.Validation