Skip to content

Conversation

@sellakumaran
Copy link
Contributor

  • Blueprint discovery: Always use displayName from a365.config.json as source of truth

    • Removed cached objectId lookup to fix bug where displayName changes were ignored
    • Simplified dual-path discovery to single displayName-first approach
    • Lines affected: BlueprintSubcommand.cs:547-578
  • JWT security: Filter Microsoft Graph tokens from console output

    • Debug logging when tokens are filtered (visible with --verbose)
    • Lines affected: CommandExecutor.cs:237-289
  • Tests: Added regression tests for both fixes

- Blueprint discovery: Always use displayName from a365.config.json as source of truth
  * Removed cached objectId lookup to fix bug where displayName changes were ignored
  * Simplified dual-path discovery to single displayName-first approach
  * Lines affected: BlueprintSubcommand.cs:547-578

- JWT security: Filter Microsoft Graph tokens from console output
  * Debug logging when tokens are filtered (visible with --verbose)
  * Lines affected: CommandExecutor.cs:237-289

- Tests: Added regression tests for both fixes
@sellakumaran sellakumaran requested a review from a team as a code owner January 9, 2026 01:24
Copilot AI review requested due to automatic review settings January 9, 2026 01:24
@sellakumaran sellakumaran requested a review from a team as a code owner January 9, 2026 01:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request addresses two important issues: blueprint idempotency when displayName changes and JWT token exposure in console output.

Key Changes:

  • Simplified blueprint discovery to always use displayName from a365.config.json as the source of truth, removing the previous objectId-first fallback approach
  • Added JWT token filtering in CommandExecutor to prevent sensitive Microsoft Graph tokens from being printed to console
  • Comprehensive regression tests for both fixes

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
CommandExecutor.cs Added IsJwtToken() method with heuristic detection and filtering logic in ExecuteWithStreamingAsync to prevent JWT tokens from being printed to console while still capturing them in StandardOutput
CommandExecutorTests.cs Added comprehensive regression tests for JWT token detection covering valid tokens, invalid formats, edge cases, null/empty handling, and integration with ExecuteWithStreamingAsync
BlueprintSubcommand.cs Changed CreateAgentBlueprintAsync to use displayName-first discovery exclusively, removing the previous objectId-first lookup to fix idempotency bug when users change displayName in config
BlueprintLookupServiceTests.cs Added regression test for displayName mismatch scenario to verify that searching by new displayName correctly returns NotFound
BlueprintSubcommandTests.cs Added note explaining removal of complex regression tests, relying on existing integration tests and service-level tests instead

mengyimicro
mengyimicro previously approved these changes Jan 9, 2026
Updated comments in CommandExecutor.cs to clarify that only standalone JWT tokens are filtered, not tokens embedded in log messages, and explained the rationale. Removed obsolete regression test comments from BlueprintSubcommandTests.cs. Simplified IsJwtToken invalid format tests in CommandExecutorTests.cs for clarity.
@sellakumaran sellakumaran enabled auto-merge (squash) January 9, 2026 01:50
@sellakumaran sellakumaran merged commit 4877438 into main Jan 9, 2026
5 checks passed
@sellakumaran sellakumaran deleted the users/sellak/setupIdempotencyFix branch January 9, 2026 18:01
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.

4 participants