Repository navigation
Add career-coach scenario: Career Coach autopilot using Node.js SDK - #338
Akshit-MSFT (Akshit0022) wants to merge 6 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Career Coach scenario under scenarios/career-coach/, expanding the repo’s scenario samples with a Node.js/TypeScript Agent 365 agent that uses SharePoint Lists + Microsoft Graph for durable plan/progress state and proactive flows.
Changes:
- Introduces a full Career Coach agent implementation (LLM path + deterministic handlers) with SharePoint-backed state, quizzes, milestones, and completion email flow.
- Adds SharePoint provisioning/seed/reset scripts plus reference CSV datasets for competency framework and learning catalog.
- Adds scenario documentation and Teams manifest/assets for sideloading/testing.
Reviewed changes
Copilot reviewed 36 out of 39 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scenarios/career-coach/.env.template | Environment variable template for OpenAI/Azure OpenAI, A365 identity/auth, SharePoint, and webhook settings |
| scenarios/career-coach/.gitignore | Scenario-local ignore rules for build artifacts, caches, state, and logs |
| scenarios/career-coach/AGENT-CODE-WALKTHROUGH.md | Walkthrough documentation for the scenario code |
| scenarios/career-coach/README.md | End-to-end setup and testing instructions for the scenario |
| scenarios/career-coach/ToolingManifest.json | MCP server manifest for tooling integration |
| scenarios/career-coach/package.json | Node/TS dependencies and scripts for dev/build and SharePoint helpers |
| scenarios/career-coach/tsconfig.json | TypeScript compiler configuration |
| scenarios/career-coach/verify-userstate.ps1 | PowerShell helper to inspect UserState list contents via Graph |
| scenarios/career-coach/SharePoint Data/CompetencyFramework_v2.csv | Seed data: role → competency framework |
| scenarios/career-coach/SharePoint Data/LearningCatalog_v2.csv | Seed data: course catalog mapped to competencies |
| scenarios/career-coach/docs/design.md | Design/architecture documentation for the scenario |
| scenarios/career-coach/images/thumbnail.png | Scenario thumbnail asset |
| scenarios/career-coach/manifest/manifest.json | Teams app manifest (placeholder IDs) |
| scenarios/career-coach/manifest/agenticUserTemplateManifest.json | Agentic user template manifest |
| scenarios/career-coach/manifest/color.png | Teams icon asset |
| scenarios/career-coach/manifest/outline.png | Teams icon asset |
| scenarios/career-coach/src/index.ts | Express host: health check, /api/messages, proactive /api/portal-event, startup wiring |
| scenarios/career-coach/src/agent.ts | AgentApplication wiring, lifecycle routing guard, message + notification handlers, adaptive card action handlers |
| scenarios/career-coach/src/client.ts | OpenAI Agents client wrapper, system prompt, SharePoint function-tools integration, observability scopes |
| scenarios/career-coach/src/cards.ts | Adaptive Card rendering utilities and payload helpers |
| scenarios/career-coach/src/career-coach-types.ts | Shared data model types + SharePoint configuration constants |
| scenarios/career-coach/src/career-coach-service.ts | Deterministic service layer: SharePoint CRUD, progress logic, grading logic, milestone aggregation |
| scenarios/career-coach/src/handlers.ts | Deterministic handlers for card submits, progress sync, quiz flows, milestones, and completion email |
| scenarios/career-coach/src/llm-tasks.ts | Focused LLM subcalls for quiz generation, short-answer grading, and completion email prose |
| scenarios/career-coach/src/openai-config.ts | OpenAI vs Azure OpenAI client configuration |
| scenarios/career-coach/src/graph-service.ts | Graph client setup (MSAL device-code for scripts + agentic token exchange for runtime), subscriptions, mail helpers |
| scenarios/career-coach/src/sharepoint-tools.ts | Graph-backed SharePoint tools exposed as OpenAI function tools (site/list ID resilience, column mapping) |
| scenarios/career-coach/src/sharepoint-column-map.ts | SharePoint display-name ↔ internal-name mapping and defensive field coercions |
| scenarios/career-coach/src/subscription-manager.ts | Create/renew Graph list change-notification subscription with local persisted state |
| scenarios/career-coach/src/proactive-refs.ts | Disk-backed mapping of AAD object IDs to proactive conversation IDs |
| scenarios/career-coach/src/file-storage.ts | Disk-backed Storage implementation for Proactive subsystem restart resilience |
| scenarios/career-coach/src/quiz-cache.ts | In-memory cache of generated quiz questions/answer keys for deterministic grading |
| scenarios/career-coach/src/token-cache.ts | Custom in-memory token cache + token resolver for observability exporter |
| scenarios/career-coach/src/scripts/setup-sharepoint.ts | Script: idempotent provisioning of SharePoint lists + columns |
| scenarios/career-coach/src/scripts/seed-reference-data.ts | Script: seed reference lists from CSVs |
| scenarios/career-coach/src/scripts/mark-courses-complete.ts | Script: simulate portal completions by inserting LearningPortalStatus rows |
| scenarios/career-coach/src/scripts/reset-milestones.ts | Script: reset milestone guards for re-demos |
| scenarios/career-coach/src/scripts/backup-and-reset-user.ts | Script: snapshot + delete a user’s rows across write-lists |
| scenarios/career-coach/src/scripts/clear-list.ts | Script: delete all rows from a specified list |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 36 out of 39 changed files in this pull request and generated 1 comment.
Suppressed comments (8)
Previously missed (4) — in code that hasn't changed since the last review.
scenarios/career-coach/verify-userstate.ps1:40
- The Graph list-items request uses
expand=fields, but Microsoft Graph expects$expand=fields. As written,item.fieldswon't be expanded and the script will fail when it tries to read$item.fields.
scenarios/career-coach/src/token-cache.ts:30 - The TokenCache comment says it has "expiration handling", but the implementation never expires entries. This is misleading for readers and callers (especially since the resolver may be hit frequently).
scenarios/career-coach/src/token-cache.ts:42 TokenCache.set()logs on every cache write. Since the observability SDK may refresh tokens frequently, this can create noisy logs and unnecessary overhead. Gate this behind an env var (or remove it).
scenarios/career-coach/src/token-cache.ts:53TokenCache.get()logs on every cache miss, which can be very frequent and flood logs in normal operation. Gate the miss log behind an env var (or remove it).
scenarios/career-coach/src/career-coach-types.ts:4
- This new TypeScript source file is missing the required Microsoft copyright header, and the opening comment still references an MCP server even though the scenario now uses Graph-backed tools.
// Career Coach data types
// These types define the structure of data in SharePoint Lists
// accessed via the SharePoint MCP Server
scenarios/career-coach/package.json:45
- This package uses Express v5 (
express: ^5.1.0) but pins@types/expressto v4. That mismatch can cause incorrect typings and TypeScript compile errors. Use the Express v5 type package version (as other samples in this repo do).
"devDependencies": {
"@microsoft/m365agentsplayground": "^0.2.18",
"@types/express": "^4.17.21",
"@types/node": "^20.14.9",
scenarios/career-coach/docs/design.md:5
- This design doc appears to be the generic OpenAI sample-agent template (title and overview) rather than Career Coach–specific content. Since the PR description points readers here for Career Coach architecture/flows, the opening section should be updated to match this scenario.
# OpenAI Sample Agent Design (Node.js/TypeScript)
## Overview
This sample demonstrates an agent built using the official OpenAI Agents SDK for Node.js. It showcases TypeScript patterns, MCP server integration, notification handling, and Microsoft Agent 365 observability.
scenarios/career-coach/AGENT-CODE-WALKTHROUGH.md:22
- The file-structure diagram is still for
sample-agent/and omits many Career Coach modules (handlers/service layer/SharePoint scripts). This makes the walkthrough misleading for contributors trying to navigate the scenario code.
sample-agent/
├── src/
│ ├── agent.ts # Main agent implementation (~60 lines)
│ ├── client.ts # OpenAI client wrapper with observability
│ └── index.ts # Express server entry point
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 38 changed files in this pull request and generated 16 comments.
Suppressed comments (2)
scenarios/career-coach/src/career-coach-types.ts:3
- This new TypeScript source file is missing the required Microsoft copyright and MIT license header.
// Career Coach data types
// These types define the structure of data in SharePoint Lists
// accessed via Microsoft Graph (agentic delegated auth)
scenarios/career-coach/src/scripts/seed-reference-data.ts:147
- Skipping whenever any item exists makes the seeder non-resumable. If one row fails after earlier rows were created, the next run sees a non-empty list and skips it, leaving the reference catalog permanently incomplete unless the developer deletes all successful rows. Seed missing rows by the stable role/course key, or verify the full expected key set before declaring the list complete.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 38 changed files in this pull request and generated 1 comment.
Suppressed comments (14)
Previously missed (9) — in code that hasn't changed since the last review.
scenarios/career-coach/src/handlers.ts:623
- This automatically Cc's the manager with the employee's target role, courses, and time investment as soon as progress reaches 100%, without any consent state or confirmation action. That contradicts the agent's own rule in
src/client.ts:101(“Never share with managers without explicit consent”) and exposes private career data. Gate manager delivery on an explicit, persisted user choice; otherwise send only to the employee.
if (managerEmail && managerEmail.toLowerCase() !== (userEmail ?? '').toLowerCase()) cc.push(managerEmail);
// Fallback: if we somehow lack a userEmail, still send to the manager only.
if (to.length === 0 && cc.length > 0) { to.push(cc[0]); cc.length = 0; }
if (to.length === 0) {
note = 'No manager or user email found — the email was drafted but not sent.';
} else {
try {
await sendMail({ to, cc, subject, htmlBody, fromUserId: userAADId, graph });
scenarios/career-coach/src/agent.ts:147
- The quiz work is started without an in-flight/idempotency guard. Two rapid clicks or a retried invoke can both read the cache before
clearQuiz, then append duplicate QuizResponses, update UserState concurrently, send duplicate cards, and potentially dispatch the completion email twice. Claim an idempotency key (for example user/course plus activity ID) before launching the deferred handler and reject or join duplicates.
// Fire-and-forget so the invoke returns fast (LLM short-answer grading + writes
// can take ~5-10s; Teams' invoke timeout is ~10s and shows a red banner if we exceed it).
handleQuizSubmit(context, this.authorization, {
userAADId,
courseId,
skillId,
answersByQuestionId,
}).catch((err) => {
console.error('[Quiz] Deferred handleQuizSubmit failed:', (err as any)?.message ?? err);
});
scenarios/career-coach/src/subscription-manager.ts:99
- Renewal matches only URL and resource, so changing
PORTAL_EVENT_SECRETkeeps the old subscription and its oldclientState. Graph then continues sending the old value while the endpoint compares against the new secret, causing every notification to return 401 until the subscription expires or is manually removed. Persist a non-reversible fingerprint of the configured clientState and recreate the subscription when it changes.
scenarios/career-coach/.gitignore:24 subscription-manager.tscreates.sp-subscription.json, but this generated file is not ignored. It contains the subscription ID, SharePoint resource identifiers, and tunnel URL, so a normal local run can accidentally commit tenant-specific runtime state. Add it to the runtime-state ignore list.
# runtime state + backups
.proactive-storage.json
.proactive-refs.json
backups/
scenarios/career-coach/src/career-coach-types.ts:3
- This TypeScript source file is the only file in
src/missing the required Microsoft copyright and MIT license header; comparesrc/agent.ts:1-2andsrc/handlers.ts:1-2. Add the standard header before the file description.
// Career Coach data types
// These types define the structure of data in SharePoint Lists
// accessed via Microsoft Graph (agentic delegated auth)
scenarios/career-coach/.env.template:26
- The Node.js sample configuration is missing the required
ENABLE_A365_OBSERVABILITYswitch and leaves the required observability log variables blank/commented. The sample template requires active defaults for all four observability settings so local and deployed behavior is explicit.
# --- A365 observability ------------------------------------------------------
# Enable the observability exporter. Default false = console-only exporter.
# MUST be true in any deployed / dev-tunnel environment or MAC Activity stays empty.
ENABLE_A365_OBSERVABILITY_EXPORTER=false
# When true, the sample demos a custom token resolver + token cache; otherwise it
# uses the built-in AgenticTokenCache from the observability hosting package.
Use_Custom_Resolver=true
# Optional exporter logging. One of 'info' | 'warn' | 'error'. Defaults to silent.
# Uncomment OTEL_LOG_LEVEL too when debugging export failures.
A365_OBSERVABILITY_LOG_LEVEL=
# OTEL_LOG_LEVEL=INFO
scenarios/career-coach/verify-userstate.ps1:10
- The comment says this configuration matches
.env, but only the host does; customSP_SITE_PATHandSP_LIST_USER_STATEvalues are ignored. Users following the README's supported rename/custom-site path will query the wrong site or list. Read all three values from the corresponding environment variables.
scenarios/career-coach/README.md:50 - The PR description says no admin consent is required, but this README correctly requires admin consent for delegated
Sites.ReadWrite.AllandUser.Read.All, andgraph-service.tsrequests those scopes. These are materially different setup requirements; update the PR description/auth claims to match the implementation and README.
**Microsoft Entra (Azure AD) app registration for the agent**
- The A365 platform provisions one for you when you create a hosted-agent project. That app's client ID + secret + tenant ID go into `.env` under `connections__service_connection__settings__*`.
- **Required delegated Graph scopes (with admin consent):**
- `Sites.ReadWrite.All`
- `User.Read.All`
scenarios/career-coach/src/index.ts:155
- Every notification rescans a ten-minute window and treats all rows in that window as affected. If user A completes a course and user B completes one five minutes later, B's notification causes both A and B to be messaged again; Graph retries produce the same duplication. Track the actual changed item/resource data when available, or persist a per-item
LastUpdatedwatermark/idempotency key so each portal update is processed once.
const usersToNotify = new Map<string, any>();
for (const row of rows) {
const f = row.fields || {};
const lastUpdated = f.LastUpdated ? new Date(f.LastUpdated) : null;
if (!lastUpdated || isNaN(lastUpdated.valueOf())) continue;
if (lastUpdated < cutoff) continue;
const aad = String(f.UserAADId || '').trim();
if (!aad) continue;
// Keep the newest row per user (avoid double-DM if a user has multiple recent changes).
const existing = usersToNotify.get(aad);
if (!existing || new Date(existing.LastUpdated ?? 0) < lastUpdated) usersToNotify.set(aad, f);
scenarios/career-coach/src/career-coach-service.ts:53
- These runtime read helpers receive an agentic Graph client, but site/list resolution calls
getSiteId()andgetListIdByName(), which each create the file-backed MSAL developer client. Consequently normal and proactive turns still require.mstoken-cache.json, fail after that cache expires or in deployment, and resolve metadata under the developer rather than the current agentic identity. Thread the supplied client through all site/list lookup helpers so the runtime path is actually agentic-only.
export async function readCompetencyFramework(graph: MsGraphClient, siteId?: string): Promise<CompetencyFrameworkRow[]> {
const site = siteId ?? await getSiteId();
const listId = await getListIdByName(site, SP_CONFIG.lists.competencyFramework);
scenarios/career-coach/src/llm-tasks.ts:100
- The prompt requests five questions, but this code only truncates the response; it accepts zero to four questions, invalid MCQ choice counts, and missing answer keys. Since passing is hard-coded as score >= 4, a short model response can make the quiz impossible to pass. Validate exactly five questions (including the 3/2 type split, four choices, and nonempty keys) and retry or use a deterministic fallback before caching/rendering it.
const json = await chatJson<{ questions: QuizQuestionWithKey[] }>(system, user, { temperature: 0.7 });
const questions = Array.isArray(json?.questions) ? json.questions : [];
// Normalize/fill missing fields defensively.
return questions.slice(0, 5).map((q, i) => ({
scenarios/career-coach/src/scripts/backup-and-reset-user.ts:106
- Each
backupAndClearListcall deletes rows immediately, while the backup file is not written until all three calls finish. A later Graph error, process interruption, or filesystem failure therefore leaves already-deleted data with no backup, contrary to the command's stated guarantee. Read all three datasets and atomically persist the snapshot first; only then begin deletions, reporting any partial cleanup separately.
lists: {
userState: await backupAndClearList(graph, siteId, SP_CONFIG.lists.userState, userAADId),
learningPortalStatus: await backupAndClearList(graph, siteId, SP_CONFIG.lists.learningPortalStatus, userAADId),
quizResponses: await backupAndClearList(graph, siteId, SP_CONFIG.lists.quizResponses, userAADId),
scenarios/career-coach/README.md:17
- The sample README does not follow the repository README template: it starts with a table of contents and omits or reorders required sections such as “What This Sample Demonstrates,” “Authentication + Identity,” “Environment Configuration,” “Deploying the Agent,” “Observability,” and “Additional Resources.” Restructure it to the mandated section order while preserving the scenario-specific setup and walkthrough details.
## Table of contents
1. [Prerequisites](#1-prerequisites)
2. [Clone + install](#2-clone--install)
3. [Configure `.env`](#3-configure-env)
scenarios/career-coach/src/index.ts:21
- Continuing after an
uncaughtExceptionis unsafe because Node may be left with partially mutated application or storage state; this handler also prevents a process supervisor from restarting the service cleanly. Log the exception and terminate with a nonzero status instead of claiming the server remains usable.
process.on('uncaughtException', (err) => {
console.error('Uncaught exception (ignored, server stays up):', err?.message ?? err);
});
9a73e6b to
179ba6f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 38 changed files in this pull request and generated 4 comments.
Suppressed comments (8)
Previously missed (1) — in code that hasn't changed since the last review.
scenarios/career-coach/src/subscription-manager.ts:176
- After a subscription expires or is deleted, renewal returns 404 forever because each interval retries the same persisted ID. The proactive flow remains disabled until the process restarts. Detect terminal renewal failures, discard the stale state, and create a fresh subscription while retaining retry behavior for transient failures.
scenarios/career-coach/src/career-coach-service.ts:53
- This read path receives an agentic Graph client, but resolves both IDs through helpers that construct the file-cached MSAL client. A deployed runtime without
.mstoken-cache.jsontherefore fails before the agentic client is used. Pass the supplied client through ID resolution (and do the same in the other runtime CRUD helpers) so runtime access is actually agentic-only.
export async function readCompetencyFramework(graph: MsGraphClient, siteId?: string): Promise<CompetencyFrameworkRow[]> {
const site = siteId ?? await getSiteId();
const listId = await getListIdByName(site, SP_CONFIG.lists.competencyFramework);
scenarios/career-coach/src/sharepoint-tools.ts:238
- These optional filters make the generic tool capable of returning every employee's
UserStateorQuizResponsesrow. Prompt instructions are not an authorization boundary, so a manipulated model call can expose private career data. Derive the caller's AAD ID fromRunCtxand enforce owner scoping for sensitive-list reads and writes in code.
scenarios/career-coach/src/agent.ts:270 - The save action has no in-flight or idempotency guard. Two clicks can run concurrent read-then-create operations, both observe no existing row, and create duplicate
UserStaterecords; later reads then select an arbitrary duplicate. Add a server-side operation/idempotency key and disable or deduplicate repeated submissions before starting this detached work.
handleSavePlanSubmit(context, this.authorization, {
userAADId, displayName, roleTitle, targetRoleId, groups, ratings,
}).catch((err) => {
console.error('[SavePlan] Deferred save failed:', (err as any)?.message ?? err);
scenarios/career-coach/src/index.ts:150
- Every notification received during this ten-minute window selects the same recently updated rows again. Graph retries or another user's update therefore resend quizzes to users already notified, while their plan remains quiz-pending. Persist a per-row/version watermark (or otherwise deduplicate notification processing) instead of using a sliding wall-clock window.
const lastUpdated = f.LastUpdated ? new Date(f.LastUpdated) : null;
if (!lastUpdated || isNaN(lastUpdated.valueOf())) continue;
if (lastUpdated < cutoff) continue;
scenarios/career-coach/src/scripts/backup-and-reset-user.ts:74
- Rows are deleted before any backup file is written. If fetching a later list, deleting a later row, creating the directory, or writing the JSON fails, earlier data has already been removed with no snapshot on disk. Fetch all three lists and atomically persist the complete backup before beginning deletion.
// Delete each row.
let ok = 0, fail = 0;
for (const row of mine) {
try {
await graph.api(`/sites/${siteId}/lists/${listId}/items/${row.id}`).delete();
scenarios/career-coach/src/llm-tasks.ts:100
- Slicing only caps the response; it does not enforce five valid questions, three MCQs, four choices, unique IDs, or nonempty answer keys. A malformed/short LLM response can render an impossible quiz while passing still requires four correct answers. Validate the complete schema and retry or fail gracefully before caching/rendering it.
const json = await chatJson<{ questions: QuizQuestionWithKey[] }>(system, user, { temperature: 0.7 });
const questions = Array.isArray(json?.questions) ? json.questions : [];
// Normalize/fill missing fields defensively.
return questions.slice(0, 5).map((q, i) => ({
scenarios/career-coach/README.md:18
- The sample README does not follow the repository README template: it omits the required standalone “What This Sample Demonstrates,” “Authentication + Identity,” “Environment Configuration,” “Deploying the Agent,” “Observability,” “Additional Resources,” “Trademarks,” and “License” sections and their prescribed order. Restructure the document while preserving the scenario-specific setup and walkthrough details.
## Table of contents
1. [Prerequisites](#1-prerequisites)
2. [Clone + install](#2-clone--install)
3. [Configure `.env`](#3-configure-env)
4. [First-time SharePoint setup](#4-first-time-sharepoint-setup)
An Agent 365 AI Teammate that helps employees set a target role, maps skill gaps, recommends courses, quizzes them proactively via Graph change notifications, and drafts the manager wrap-up email. Hybrid pro-code: deterministic TypeScript for all card submits/writes/business rules; focused LLM sub-calls for conversation, quiz generation, grading, and email prose. SharePoint Lists for state, Microsoft Graph (agentic delegated) for triggers + mail. Includes Copilot review fixes: idempotent quiz submit, no-tools email summarization (prompt-injection safe), real token usage, admin-consent scope docs. Placeholder GUIDs; no secrets or tenant-specific values.
72ea808 to
bd4e262
Compare
Akshit-MSFT (Akshit0022)
left a comment
There was a problem hiding this comment.
I’ve updated the PR based on all the review points and addressed the comments.
…, WorkIQ tools Brings the career-coach scenario up to the current Agent 365 SDK and observability pattern (matches microsoft#339), and adds the features built since the first revision. Observability - Migrate from the preview @microsoft/agents-a365-observability* packages to the @microsoft/opentelemetry distro (src/telemetry.ts). - Export over the S2S route with an app-only token for the runtime agent identity (src/observability/app-token-resolver.ts: blueprint FMI assertion -> agent identity -> Observability API; validated, cached, never logged). Removes the delegated token cache (src/token-cache.ts). - gen_ai.agent.id comes from activity.recipient.agenticAppId; blueprint id is metadata only. invoke_agent / chat / execute_tool spans on every path (LLM turns, card actions, typed shortcuts, email, proactive), with channel, caller and a stable conversation id (email threads). Features - Email channel: agent converses by email (org members only, bounces ignored), thread-scoped history, "Save it" saves the proposed plan via the same code as the Teams button (email-conversation.ts, email-plan-store.ts). - Goal lifecycle: one UserState row per goal (Active / Completed / Replaced) with history; completion marks a goal Completed only after the email is sent; "show my previous goals" tool. - WorkIQ MCP (Mail, Calendar V2 servers in ToolingManifest.json); WorkIQ SharePoint is read-only so list writes stay behind the code guards. - Write guards: LLM cannot mark progress, edit finished goals, create a second active goal, or switch role on a goal with progress; SharePoint tools are owner-scoped to the caller. - Reliability: card actions run deferred with user-visible failure handling and safe card unlock; typed shortcuts for sync / completions; LLM retries with history rollback. Packages: @microsoft/agents-a365-* 2.x, @microsoft/opentelemetry, @openai/agents 0.7, openai 6, zod 4; npm overrides no longer needed. Docs: README keeps the template sections and is updated for the new architecture (auth, deploy, troubleshooting); docs/design.md and the code walkthrough rewritten; .env.template regenerated. Removed: verify-userstate.ps1 (debug helper no longer used). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The recent update brings the Career Coach up to the current Agent 365 SDK and observability pattern, and adds the features built since the first revision. The PR description has been refreshed to match. 1. Observability — S2S route with an app-only token (matches #339)
2. New / upgraded features
3. Package upgrades
The npm 4. Docs and config
5. Note on an earlier review threadThe email-injection thread ( Not included (local / generated)
Verification
Live-tenant validation evidence (E2E, Defender, Purview, admin center) will follow in a separate comment. |
Validation evidence: Career Coach scenario (A365 observability + WorkIQ MCP)The agent under test ran locally behind a dev tunnel from a build of the same 1. E2E playbook (AutoQAgent): 5 / 5 passed
Run finished 2026-10-22 16:42:23Z (duration 8m 46s).
2. Runtime exporter evidence (agent log, redacted aggregates)Playbook window (≈16:33:37Z → 16:42:23Z):
Whole agent session (2026-10-22 15:30Z → 18:00Z): 10 user turns; 27 / 27 export groups succeeded, all for the same runtime Agent Identity (no blueprint-tagged groups); 101 genAI spans delivered; 0 export failures; no app-only token acquisition failures. In every batch (36 / 36) the skipped spans are exactly the non-genAI spans the exporter filters out (log: Five 3. A365 code validator: ✅ PASS
4. Build and repository checks on
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved write-authorization gaps and concurrent state-update risks can expose private reports or corrupt saved progress.
11 open issues
Protect plan mutations from concurrent updates · New Prevent model-controlled completion recipients · New Restrict writes to authorized configured lists · New Validate column aliases before write guards · New Bind created plans to the authenticated caller · New Verify plan ownership before updates · New Use the runtime Graph client for list discovery · New Keep rendered quiz answer keys immutable · New Preserve explicit field clears on updates · New Validate subscription secrets before renewal · New Recreate expired or deleted subscriptions · New
7 resolved since last review
The quiz action is also replayable: a double-click starts two grading pipelines before either… Continuing after anuncaughtExceptioncan leave Node.js state corrupted while the health endpoint… The email body is untrusted sender content, yet this prompt explicitly tells the tool-enabled agent… The generated.sp-subscription.jsonis not ignored even though it contains tenant-specific… The PR states that setup needs no admin consent, but this device-code flow explicitly requests… These constant token counts make every observability span report fabricated usage, which corrupts… The sample README does not follow the repository README template: required sections such as “What…
Review effort: Balanced
…tions) SharePoint tools (sharepoint-tools.ts, sharepoint-column-map.ts) - Restrict chat writes to the configured UserState list; the data tools only operate on the configured Career Coach site and only read the five Career Coach lists. Reference data, portal completions and quiz audit rows are written only by provisioning, the portal and the quiz grader. - Scope personal lists (UserState, LearningPortalStatus, QuizResponses) to the caller's own rows in code; bind new plans to the authenticated caller and reject a different UserAADId; verify row ownership fail-closed before an update and never allow the owner to change. - Canonicalize model field names to display names (case-insensitive, internal field_N included) before every guard; reject conflicting aliases. - Make ManagerName / ManagerEmail app-managed so the model cannot redirect the completion email. Concurrency (user-lock.ts, handlers.ts, quiz-cache.ts) - Serialise every UserState read-modify-write per user: save, sync, quiz grading, milestone / completion persistence and the model's create / update. - Share one in-flight quiz generation per user/course so a card is always graded against its own answer key. Runtime auth and data (graph-service.ts, career-coach-service.ts) - Pass the runtime Graph client through site / list discovery; the developer device-code client is now only used by scripts and background jobs. - Preserve intentional clears on trusted app updates (null for optional columns; required columns are never cleared). Subscriptions (subscription-manager.ts) - Store a hash of PORTAL_EVENT_SECRET with the saved subscription and replace (delete + create) it when the secret, URL or resource changes. - Re-create the subscription when renewal reports it gone or its saved expiry has passed, instead of retrying the dead id forever. Prompt (client.ts): the model is told chat writes are limited to UserState and that the app records quiz results, audit rows and manager details. Docs: guard tables, security table, file maps and trade-offs updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The dependency-review check flags @azure/msal-node 5.x: "authorization code flow may disclose parameters to an attacker-controlled endpoint" (high; fixed in 7.1.0). The sample only uses PublicClientApplication with the device-code and silent flows for the setup / seed scripts; both work unchanged on 7.1.1 (type-check clean, silent flow and API surface verified). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed all 11 open threads from the review of Review threads
|
Rick Brighenti (rbrighenti)
left a comment
There was a problem hiding this comment.
Found four correctness issues: two P1 issues involving backup data loss and goal lifecycle races, and two P2 issues involving stale quiz submissions and duplicate completion emails. Details are inline. All four were reproduced against unchanged source with mocked I/O; live tenant behavior was not verified.
| for (const row of mine) { | ||
| try { | ||
| await graph.api(`/sites/${siteId}/lists/${listId}/items/${row.id}`).delete(); | ||
| ok++; |
There was a problem hiding this comment.
[P1] Persist the complete backup before deleting any rows
backupAndClearList deletes rows immediately, but main processes all three lists before writing the snapshot. If a later Graph read or the final local backup write fails, already-deleted rows have no persisted backup. Reproduced with all three lists deleted followed by an EACCES from writeFileSync. Collect and successfully persist the complete snapshot before issuing any DELETE requests.
| await upsertUserState(graph, { | ||
| ...record.state, | ||
| Completion100Fired: true, | ||
| PlanStatus: 'Completed', |
There was a problem hiding this comment.
[P1] Complete the originating goal rather than whichever goal is active
The completion email is composed/sent outside the user lock, after which this path re-reads the currently active goal and marks it Completed. If the user saves a new role while that email is in flight, the new goal is closed instead of the original. Reproduced by pausing composition, saving a replacement role, then releasing composition: the new goal became Completed at 0% and the old goal remained Replaced. Carry the originating UserState item ID/version through the operation and verify it before updating.
| // Counted only once the reads succeed, so a network failure above isn't an attempt. | ||
| const attempts = bumpAttempts(input.userAADId, input.courseId) || 1; | ||
| const lp = userRecord.state.LearningProgress.find((l) => l.courseId === input.courseId); | ||
| const courseTitle = lp?.courseTitle ?? course.Title; |
There was a problem hiding this comment.
[P2] Reject quiz submissions that do not belong to the current goal
The optional lp?.courseTitle fallback lets a submission proceed even when no matching LearningProgress entry exists. Cached quizzes are keyed only by user/course and survive goal replacement, so submitting an old quiz after saving another goal can call applyQuizPass against the new goal and raise shared skills even though the course is absent from its plan. Reproduced a skill increase from 0 to 3 while the only current course remained Recommended. Bind attempts to their originating goal and reject absent plan courses before any writes.
| await fireMilestone80(context, authorization, userAADId, updatedState); | ||
| } | ||
| if (updatedState.OverallProgress === 100 && !updatedState.Completion100Fired) { | ||
| await fireCompletion100(context, authorization, userAADId, updatedState); |
There was a problem hiding this comment.
[P2] Atomically claim completion before sending the email
The completion flag is checked on a snapshot outside the lock and only persisted after email delivery; there is no in-flight completion claim. Overlapping progress checks or portal syncs for the same 100%-complete, unsent goal can therefore each send its completion email. Reproduced two concurrent handleSyncProgress calls producing two sendMail calls for one goal. Claim completion atomically per goal across all entry points, allowing retries after a failed send.
Krishnadheeraj (DheerajPannala)
left a comment
There was a problem hiding this comment.
The layering is clear and the SharePoint write guards are in the right place.
18 things to address before this merges; details inline.
| const courseId = String(lp?.courseId ?? ''); | ||
| if (!courseId || courses.some((c) => c.courseId === courseId)) continue; |
There was a problem hiding this comment.
Should fix: Reject missing course IDs instead of skipping the progress guard
A learning record without a course ID bypasses the completion check, but it is still written to SharePoint. The progress calculation subsequently counts that record solely by skill and Complete status. This permits an unvalidated record to drive a goal to 100% and trigger its completion email without a passed quiz.
Evidence
scenarios/career-coach/src/sharepoint-tools.ts:311 executes 'if (!courseId ... ) continue' before checking status; updateListItem then writes the original fields at 611-614. For an active plan with one ai-engineering goal, LearningProgress=[{"courseId":"","skillId":"ai-engineering","courseTitle":"Unvalidated","status":"Complete"}] passes this guard. scenarios/career-coach/src/career-coach-service.ts:336-338 computes total=1, done=1, progressPct=100 without checking courseId or quizResult. The next handleSyncProgress recomputes that state and calls fireCompletion100 at scenarios/career-coach/src/handlers.ts:420-421.
Suggested fix: Require every learning record to have a valid plan/catalog course ID before evaluating or writing progress. Reject malformed records rather than silently excluding them from the guard.
| guardUserStateWrite(ctx, displayFields, stored); | ||
| const internalFields = toInternalFields(displayFields, colMap); | ||
| try { | ||
| const updated = await graph.api(`/sites/${siteId}/lists/${listId}/items/${itemId}/fields`).update(internalFields); |
There was a problem hiding this comment.
Minor: Preserve earned progress when applying conversational updates
The lock protects this PATCH, but not the earlier read used to construct its replacement arrays. If a quiz passes between that read and this update, the stale LearningProgress array can replace Complete with Recommended and remove the persisted quiz result. Re-reading the row for ownership and guard checks does not help because the freshly read progress is never merged into the write.
Evidence
scenarios/career-coach/src/sharepoint-tools.ts:574-579 reads the current row under the lock, but 612-614 serializes and writes displayFields supplied earlier. At 312-314, an already validated stored course is explicitly skipped by the guard; decreasing skill levels is also allowed by the comparison at 325-328. Sequence: listListItems returns course A as Recommended; handleQuizSubmit persists A as Complete with a passed quiz at scenarios/career-coach/src/handlers.ts:754-759; a later updateListItem submits the earlier array. The update passes its checks and overwrites the earned completion.
Suggested fix: Merge only permitted changes into the freshly loaded row and preserve server-owned quiz results, completion states, and earned levels. Alternatively, require the originating row version and reject stale replacement snapshots.
| } catch (err) { | ||
| console.warn('[QuizSubmit] Short-answer grading failed; marking short answers wrong:', (err as any)?.message ?? err); |
There was a problem hiding this comment.
Minor: Do not record a grading outage as incorrect answers
When short-answer grading fails, the handler continues with both answers marked incorrect and persists a failed attempt. For the normal three-MCQ/two-short quiz, even perfect answers can then score at most 3/5. This turns a dependency outage into a false assessment and adds spurious weak-topic history instead of leaving the attempt retryable.
Evidence
scenarios/career-coach/src/handlers.ts:684-689 initializes short-answer results with correct:false. The catch at 708-710 logs the grading failure but continues to summarizeGrading at 713, appendQuizResponse at 736-747, and the UserState update at 754-759. With three correct MCQs and a timeout from gradeShortAnswers, score=3 and passed=false regardless of the submitted short answers. Missing grading results are similarly converted to incorrect in scenarios/career-coach/src/llm-tasks.ts:181-188.
Suggested fix: Abort the attempt when grading is unavailable or incomplete, preserve the submitted answers for retry, and only increment attempts or persist assessment results after all required grades are valid.
| footer, | ||
| }); | ||
|
|
||
| markGraded(input.userAADId, input.courseId); |
There was a problem hiding this comment.
Minor: Mark an attempt recorded before sending its result card
The attempt and plan are already persisted when the result card is sent, but the submitted marker is set only afterward. If card delivery fails, the recovery path treats the recorded attempt as safe to resubmit and unlocks it. Retrying then appends another attempt and grades the same submission again.
Evidence
scenarios/career-coach/src/handlers.ts:736-759 writes QuizResponses and UserState before awaiting sendCard at 777-786. A rejection there skips markGraded at 788 and clearQuiz at 794. scenarios/career-coach/src/agent.ts:249-251 restores the quiz whenever wasGraded is false and cached questions remain, which is exactly this state. The gradingInFlight claim is released in handlers.ts:657-659, so the restored submit enters the write path again.
Suggested fix: Set the recorded-attempt marker immediately after successful persistence, before any outbound card delivery. A delivery failure must offer recovery of the result, not permission to record the same attempt again.
| } catch (err: any) { | ||
| const msg = err?.message ?? String(err); | ||
| console.error(`[sub-mgr] ✗ Failed to create subscription: ${msg}`); | ||
| console.error('[sub-mgr] Hint: make sure PORTAL_WEBHOOK_URL is reachable from the internet and the /api/portal-event route is running (Graph validates it during CREATE).'); | ||
| return null; |
There was a problem hiding this comment.
Minor: Retry subscription creation after an initial startup failure
A transient failure during initial subscription creation disables proactive notifications until the process is restarted. The server continues serving normally, but no recovery timer has been started. Restoring Graph connectivity or bringing the tunnel online later cannot recover this feature automatically.
Evidence
scenarios/career-coach/src/index.ts:215-218 calls ensureSubscription once during startup. In scenarios/career-coach/src/subscription-manager.ts, startRenewTimer is called only after successful renewal or creation at 132 and 177; the initial create failure returns null at 179-183. Even starting the existing timer alone would not recover an empty installation because its callback returns immediately when no state file exists at 198-199. A first POST /subscriptions returning 503 therefore leaves no subscription and no subsequent creation attempt.
Suggested fix: Start a bounded-backoff recovery loop independently of initial creation success. When no usable subscription state exists, the loop should retry ensureSubscription rather than returning without work.
| # Random shared secret. Generate one with: | ||
| # [Guid]::NewGuid().ToString() + [Guid]::NewGuid().ToString() | ||
| PORTAL_EVENT_SECRET=<CHANGE_ME_random_guid_pair> |
There was a problem hiding this comment.
Minor: Shipped PORTAL_EVENT_SECRET placeholder is accepted as a valid webhook secret
The template ships PORTAL_EVENT_SECRET=<CHANGE_ME_random_guid_pair>, which is a non-empty value. The webhook (index.ts) and ensureSubscription only fail closed when the secret is empty. A .env copied from the template therefore runs normally with a secret that is published in this repo, and nothing tells the developer it was never changed.
/api/portal-event is registered before authorizeJWT and is exposed through the --allow-anonymous dev tunnel (README §5.1). Anyone who learns the tunnel URL can send the placeholder as X-Portal-Secret. The agent then DMs any employee who has chatted with it, using preface text built from attacker-chosen CourseId/Status values. The response also reveals whether a given AAD object ID has a cached conversation. The same value works as a Graph clientState, so it can be used to trigger notification processing again and again.
README line 164 understates the risk by describing the value as 'Any random string (used for a manual POST test path)'. It is also the only check on Graph notifications. The repo guidance asks to 'Confirm required secrets are validated at startup with fail-fast behavior (no silent defaults for security-critical config)'.
Evidence
.env.template:71containsPORTAL_EVENT_SECRET=<CHANGE_ME_random_guid_pair>. No code in the sample checks forCHANGE_ME; a grep over the career-coach.ts,.js,.jsonand.mdfiles returns no matches.src/index.ts:82-87:const expected = process.env.PORTAL_EVENT_SECRET || ''; if (!expected) { ... res.status(500) ... }. Only an empty value is rejected.src/index.ts:111-124: comparesreq.headers['x-portal-secret']withexpected, then callsagentApplication.handleProactivePortalEvent(aad, body)and returns its{ok, reason}as JSON.src/index.ts:95-108: accepts a Graph-shaped body whenfirst.clientState === expectedand runshandleGraphNotification.src/subscription-manager.ts:79-87: also rejects only an emptyclientState, so the Graph subscription is created with the placeholder.src/agent.ts:788-792: buildsprefacefrompayload.CourseIdandpayload.Status, checking only that they are strings.src/agent.ts:813-816: passesprefacetosendPreface, which sends it verbatim withctx.sendActivity(MessageFactory.text(text))(agent.ts:46-47).src/agent.ts:779-783: returnsNo cached conversation for aadObjectId=…for unknown users.README.md:221:devtunnel create career-coach-tunnel --allow-anonymous.PACKET.md:955(security-compliance guidance): 'no silent defaults for security-critical config'.PACKET.md:1026: 'All samples should fail fast on missing required security configuration'.- Sibling samples reject placeholder secrets explicitly, e.g.
nodejs/openai/sample-agent/src/observability-token-service.ts:52-58.
Scenario:
- A developer fills in the Azure, SharePoint and connection values but leaves line 71 as shipped. Everything works.
- An attacker sends
POST https://<tunnel>/api/portal-eventwith headerX-Portal-Secret: <CHANGE_ME_random_guid_pair>and body{"UserAADId":"<victim-oid>","CourseId":"<arbitrary text or link>"}. - The victim receives a Teams DM from Career Coach containing that text, and a sync run follows.
Suggested fix: Ship PORTAL_EVENT_SECRET= empty, so the existing empty-secret guards stay closed until a real value is generated. Alternatively, reject placeholder values (for example, anything containing CHANGE_ME) in both index.ts and ensureSubscription. Also update README line 164 to say the value is the Graph clientState as well as the header for manual tests.
| # Random shared secret. Generate one with: | |
| # [Guid]::NewGuid().ToString() + [Guid]::NewGuid().ToString() | |
| PORTAL_EVENT_SECRET=<CHANGE_ME_random_guid_pair> | |
| # Random shared secret (required). Used as the Graph subscription clientState and as the | |
| # X-Portal-Secret header for manual tests; the webhook refuses requests while it is empty. | |
| # Generate one with: | |
| # [Guid]::NewGuid().ToString() + [Guid]::NewGuid().ToString() | |
| PORTAL_EVENT_SECRET= |
| # ============================================================================= | ||
| # 2. MCP + OBSERVABILITY | ||
| # ============================================================================= | ||
| BEARER_TOKEN=<CHANGE_ME_bearer_token_optional_for_MCP_tools> |
There was a problem hiding this comment.
Minor: BEARER_TOKEN placeholder replaces the agentic MCP token, so WorkIQ tools never attach
The template ships BEARER_TOKEN=<CHANGE_ME_bearer_token_optional_for_MCP_tools>. attachWorkIqTools passes process.env.BEARER_TOKEN || '' to addToolServersToAgent, so on every turn this non-empty placeholder is used as the MCP auth token. The agentic user token is only obtained when that argument is empty, so it is never fetched.
The README describes the variable as an optional dev override ('Leave empty normally'), so most people will leave the shipped value alone. Tool-server registration or connection then fails. The error is caught and only logged as a warning, and the Mail and Calendar tools silently never attach. As a result, 'email me my plan' and 'find time for my 1:1' (README §8) don't work. The sibling Node templates ship this key empty.
Evidence
.env.template:28:BEARER_TOKEN=<CHANGE_ME_bearer_token_optional_for_MCP_tools>.src/client.ts:413:await toolService.addToolServersToAgent(this.agent, ctx.authorization, this.authHandlerName, ctx.turnContext, process.env.BEARER_TOKEN || '');. A non-empty value is passed through as the token.src/client.ts:414-418: catches the failure, logs[WorkIQ] Failed to register MCP tool servers (continuing without them)and setsthis.agent.mcpServers = [].README.md:149: 'Token froma365 develop get-token, only for running MCP tools without the platform token. Leave empty normally. Tokens expire.'AGENT-CODE-WALKTHROUGH.md:224: 'WorkIQ (dev only) |BEARER_TOKEN'.- Sibling templates use
BEARER_TOKEN=:nodejs/openai/sample-agent/.env.template:21andnodejs/claude/sample-agent/.env.template:11.
Scenario: copy the template, fill in the values the README calls required, then ask the agent 'email me my plan'. No Mail tool is available, and the only sign of the problem is the WorkIQ warning in the log.
Suggested fix: Ship BEARER_TOKEN= empty, with a comment that it is an optional, dev-only override from a365 develop get-token.
| BEARER_TOKEN=<CHANGE_ME_bearer_token_optional_for_MCP_tools> | |
| # Optional, local dev only: token from `a365 develop get-token`. Leave empty to use the agentic token. | |
| BEARER_TOKEN= |
| 3. Reconcile the messaging endpoint: `a365 setup blueprint --update-endpoint <url>/api/messages --m365`. | ||
| 4. Package with `a365 publish --agent-name "Career Coach" --aiteammate`, upload the package in the Microsoft 365 admin center, confirm the Teams Developer Portal configuration, and request an instance ([create an instance](https://learn.microsoft.com/microsoft-agent-365/developer/create-instance)). | ||
|
|
||
| Set the cloud env vars (Azure OpenAI, agentic auth, observability, SharePoint) at the platform level — not just in a local `.env`. The quiz answer-key cache is in memory and the proactive map, email-plan store and subscription state are local files, which suits a single instance; move them to shared storage before running several instances. |
There was a problem hiding this comment.
Minor: Deployment docs omit that the real-time trigger runs on the developer's device-code token cache
README §12 (line 541) says the MSAL device-code credential is used only by the setup and seed scripts. §4.4 (line 207) says that at runtime the agent works with SharePoint as its own agent user, not as the developer.
In fact, the running server uses the developer's cached device-code token (.mstoken-cache.json) for all of Feature 1:
- creating and renewing the Graph subscription, at startup and on the renewal timer;
- resolving the site and list, and reading LearningPortalStatus rows, for every change notification.
§13 points people to App Service, Container Apps or Functions and lists the local-state caveats, but not this one. A hosted instance has no cache file: it is gitignored and only created by an interactive sign-in. As a result:
ensureSubscriptiononly logs a non-fatal warning;- notifications can't be processed;
- the real-time proactive quiz silently never fires.
The same failure happens locally once the developer's refresh token lapses. The server also holds an admin's delegated Sites.Manage.All / User.Read.All / Mail.Send token, which the 'three separate credentials' description doesn't mention.
Evidence
src/index.ts:214-219: runsensureSubscription()on listen; a failure only logs[startup] ensureSubscription failed (non-fatal).src/subscription-manager.ts:94-96: callsgetSiteId()andgetListIdByName(siteId, listName)with no client, so both fall back togetGraphClient()(graph-service.ts:222andgraph-service.ts:325).createSubscription,renewSubscription,listSubscriptionsanddeleteSubscriptionalways usegetGraphClient()(graph-service.ts:362-393).src/index.ts:131-160(handleGraphNotification): callsgetSiteId(),getListIdByName(siteId, …)andgetListItems().getListItemshard-codesgetGraphClient()(graph-service.ts:338-339).graph-service.ts:125-131: throws 'No cached Microsoft account found. Runnpm run setup:sharepointonce to sign in.' when the cache is missing..gitignore:15: ignores.mstoken-cache.json.graph-service.ts:62: 'Simple file-backed MSAL cache. Not encrypted — for local dev only.'README.md:541: 'Setup / seed scripts use the MSAL device-code flow…'.README.md:207: 'At runtime the agent reads/writes the lists as its own agent user … not as the developer who ran the setup scripts.'README.md:554: lists only the quiz cache, proactive map, email-plan store and subscription state.- By contrast,
AGENT-CODE-WALKTHROUGH.md:40correctly says 'MSAL device-code for scripts and background jobs'.
Scenario: deploy per §13 with the platform env vars set, then run mark:complete. The startup log shows ensureSubscription failing on the missing cache, so no subscription exists and no proactive quiz DM ever arrives.
Suggested fix: In §4.4 and §12, state that the Feature 1 webhook (subscription lifecycle and notification row reads) runs as the developer who completed the device-code sign-in. In §13, add that a hosted instance needs that token cache, provisioned securely, or app-only/agentic replacements for these calls. Without either, only the manual POST path and user-initiated syncs work.
| | Feature 4 email "Access is denied" | `sendMail` using `/users/{id}/sendMail` on a delegated token | Should be fixed — `graph-service.ts` uses `/me/sendMail` whenever a graph client is passed in. | | ||
| | `Failed to acquire token silently` on script start | Cached MSAL token expired (60 days) or was for a different tenant | Delete `.mstoken-cache.json` and re-run — a fresh device-code prompt will appear. | | ||
| | SharePoint 500 on write with no error detail | Column-name mismatch between display and internal | Check the `[createListItem] Known columns:` log line — the field you sent must be in there. | | ||
| | Graph subscription creation fails with "Notification URL invalid" | `PORTAL_WEBHOOK_URL` isn't reachable | Verify the dev tunnel is running and the URL in `.env` matches; test with `curl <URL>?validationToken=hi` (should echo `hi`). | |
There was a problem hiding this comment.
Minor: Webhook validation test in Troubleshooting sends a GET, which the endpoint never answers
curl <URL>?validationToken=hi sends a GET, and so does Windows PowerShell's curl alias (Invoke-WebRequest). /api/portal-event is registered only for POST. A GET falls through to authorizeJWT and gets a 401, or a 404 when NODE_ENV=development. It never echoes hi, even when the tunnel and URL are correct, so this check sends people looking for a problem that isn't there. Graph's own validation request is a POST, as line 236 already says.
Evidence
src/index.ts:72:server.post('/api/portal-event', …)is the only handler for that path, and the echo happens inside it (index.ts:74-80).src/index.ts:194:server.use(authorizeJWT(authConfig))handles everything else, and there is noserver.get('/api/portal-event').README.md:236: 'Graph validates by POSTing?validationToken=X'.
Suggested fix: Use a POST in the test command.
| | Graph subscription creation fails with "Notification URL invalid" | `PORTAL_WEBHOOK_URL` isn't reachable | Verify the dev tunnel is running and the URL in `.env` matches; test with `curl <URL>?validationToken=hi` (should echo `hi`). | | |
| | Graph subscription creation fails with "Notification URL invalid" | `PORTAL_WEBHOOK_URL` isn't reachable | Verify the dev tunnel is running and the URL in `.env` matches; test with `Invoke-RestMethod -Method Post "<URL>?validationToken=hi"` (should return `hi`). | |
|
|
||
| You'll need each test user's **UPN** and **AAD Object ID** (Entra ID → Users → select the user → *Object ID*). | ||
|
|
||
| > **Set the default user for scripts:** every helper script (`mark:complete`, `reset:milestones`, `backup:user`) accepts a user AAD ID. To avoid retyping it, set env vars **once** per terminal session: |
There was a problem hiding this comment.
Minor: TEST_USER_* is documented as the default for all helper scripts, but only mark:complete reads it
§7 says every helper script (mark:complete, reset:milestones, backup:user) accepts a user AAD ID, and that setting TEST_USER_AAD_ID / TEST_USER_NAME saves retyping it. .env.template:125-126 makes the same claim. It actually works the other way round:
reset:milestonesandbackup:userread onlyprocess.argv, and exit with a usage error if the ID is missing.mark:completehas no user argument at all; its arguments are course IDs. It reads onlyTEST_USER_AAD_ID, and silently falls back to the all-zero GUID when that is unset.
Someone who treats the env vars as an optional convenience and runs §8.3's npm run mark:complete -- CS007560 writes completion rows for 00000000-…. The script reports success, but no quiz DM ever arrives.
Evidence
src/scripts/reset-milestones.ts:23-27:const userAADId = (process.argv[2] || '').trim();, then on a missing value prints 'Usage: npm run reset:milestones -- …' and callsprocess.exit(2).src/scripts/backup-and-reset-user.ts:86-90: same pattern; it never readsTEST_USER_*.src/scripts/mark-courses-complete.ts:34:const USER_AAD_ID = process.env.TEST_USER_AAD_ID || '00000000-0000-0000-0000-000000000000';src/scripts/mark-courses-complete.ts:56:const courseIds = process.argv.slice(2)….README.md:347:npm run mark:complete -- CS007560passes no user.src/index.ts:163-188: proactively notifies the row'sUserAADId, which logs 'No cached conversation for aadObjectId=00000000-…'.
Suggested fix: Say that mark:complete requires TEST_USER_AAD_ID (from .env or the shell), and that the other two scripts take the ID as an argument. Update the .env.template comment to match, or make the scripts fall back to the env vars.
| > **Set the default user for scripts:** every helper script (`mark:complete`, `reset:milestones`, `backup:user`) accepts a user AAD ID. To avoid retyping it, set env vars **once** per terminal session: | |
| > **Set the test user for `mark:complete`:** `mark:complete` has no user argument — it writes rows for `TEST_USER_AAD_ID` / `TEST_USER_NAME` (from `.env` or the shell), so set them before §8.3. `reset:milestones` and `backup:user` take the AAD ID as their first argument instead: |




Add career-coach scenario: Career Coach autopilot using Node.js SDK
What this adds
A scenario extension under
scenarios/career-coach/- a private AI Career Coach built on the Microsoft Agent 365 SDK + OpenAI Agents SDK that runs an employee's growth journey end-to-end as an Agent 365 AI Teammate (its own M365 identity and mailbox), in Microsoft Teams and by email: goal setting, skill-gap mapping, course recommendations, proactive post-course quizzes, milestone nudges, and a manager wrap-up email. Complements the Chief-of-Staff (#333) and Scrum-Master (#334) scenarios by covering the talent-development / employee-growth side of leader / squad workflows.Capabilities
Each flow is a deterministic handler in
src/handlers.ts, with focused LLM sub-calls insrc/llm-tasks.ts:CompetencyFramework_v2+ course recommendations fromLearningCatalog_v2(planReview card)UserState, one row per goal (Active/Completed/Replaced) so finished goals are kept as historyLearningPortalStatusagainst the plan and recomputes progressFull flows, data model and design decisions:
scenarios/career-coach/docs/design.mdandAGENT-CODE-WALKTHROUGH.md.Design principles
Action.Executehandler acks fast and runs deferred; cards lock after use and in-flight claims absorb double clicks.What's included
Local testing
The conversational path (goal setting, skill ratings, gap analysis, 1:1 prep) runs locally against the Agents Playground:
npm install,npm run dev,npm run test-tool. The full data-backed loop (save plan, proactive quiz, milestones, completion email, email channel) requires:gpt-4orecommended)npm run setup:sharepoint), and the agent user added as a site Membera365 setup all --aiteammate,a365 setup blueprint --update-endpoint <url>/api/messages --m365,a365 publish --aiteammate, then an approved, licensed agent instanceAuth model
/meis the Career Coach agent user and mailbox). SharePoint, the completion email and WorkIQ MCP use this identity. No application-permission client secret is used for Graph.gen_ai.agent.idis the runtime identity from the turn; the blueprint id is metadata only.Sites.Manage.All,Sites.ReadWrite.All,User.Read.All,Mail.Send,offline_access).Sites.Manage.AllandUser.Read.Allare admin-restricted, so a tenant/SharePoint admin performs - or pre-consents - the one-time sign-in. The blueprint'sSites.ReadWrite.Allgrant and the WorkIQ MCP grants (a365 setup permissions mcp) also need a Global Administrator.Observability
@microsoft/opentelemetrywith the Agent 365 exporter on the S2S route. Every path - LLM turns, card actions, typed shortcuts, email and proactive turns - emitsinvoke_agent(input/output messages),chat(model/provider, finish reason) andexecute_toolspans with caller, channel, session and conversation id. OpenAI Agents auto-instrumentation adds token usage; no fabricated token counts.Verification
npm install(no lockfile) →npm run buildandnpx tsc --noEmitpass.a365-code-validatorstatic scan: 0 findings.00000000-...,contoso.sharepoint.com,<CHANGE_ME_...>etc.Placement - why
scenarios/Placed at the repo root under
scenarios/career-coach/rather than under a language tier (e.g.nodejs/openai/) because a career-coach workflow doesn't map cleanly to a single SDK - it composes the Agent 365 SDK, OpenAI Agents SDK, SharePoint, and Microsoft Graph. This mirrors how the Chief-of-Staff (#333) and Scrum-Master (#334) samples are placed. Happy to relocate tonodejs/openai/scenarios/career-coach/if reviewers prefer a language-tier scoping.Not included / future work
.mstoken-cache.json, gitignored).tests/e2e/. Recommend a follow-up PR once initial review lands.Related
nodejs/openai/sample-agent