Greptile Summary

This PR addresses review feedback on the federated credentials and certificate auth feature (#40884): it makes appPassword optional, derives MSTeamsAuthType from a single source of truth, adds missing Zod schema fields that were blocking configs from validating, restores env var names in error hints, and adds auth-material presence checks for the new auth types.

Key changes:

• MSTeamsCredentials.appPassword is now optional; authType is always present, defaulting to "clientSecret".
• hasConfiguredMSTeamsCredentials and resolveMSTeamsCredentials dispatch on authType to check/resolve the correct auth material.
• MSTeamsConfigSchema now includes authType, certPemFile, certKeyFile, sendX5C, ficClientId, widAssertionFile — configs using these fields were previously rejected at validation.
• buildMSTeamsAuthConfig in sdk.ts switches on authType to populate the SDK config object correctly.

Potential issue found:

• In sdk.ts, the certificate branch assigns base.certPemFile = creds.certPemFile and base.certKeyFile = creds.certKeyFile unconditionally — even when those values are undefined (e.g., env-only cert configs). This is inconsistent with the federated branch which uses if (creds.ficClientId) guards. If the SDK checks 'certPemFile' in config rather than truthiness, this could prevent the SDK's own env var fallback, silently breaking cert auth when only env vars are used.

Confidence Score: 3/5

• Mostly safe to merge; one logic inconsistency in sdk.ts may break certificate auth when cert files are supplied via env vars only.
• The schema, type, and error message changes are clean and correct. The resolveMSTeamsCredentials refactor is well-structured and well-tested. However, buildMSTeamsAuthConfig in sdk.ts unconditionally assigns certPemFile/certKeyFile as undefined when env-only cert config is used, unlike the federated case which uses conditional guards. Whether this breaks runtime cert auth depends on whether the SDK checks 'certPemFile' in config (broken) or falls back on truthiness (fine). The existing tests for the env-only cert path only assert authType, not end-to-end SDK behaviour, so this gap is not caught.
• extensions/msteams/src/sdk.ts — the certificate branch of buildMSTeamsAuthConfig

This is a comment left during a code review.
Path: extensions/msteams/src/sdk.ts
Line: 22-25

Comment:
**Unconditional undefined assignment may block SDK env var fallback**

`certPemFile` and `certKeyFile` are assigned to `base` unconditionally, including when they are `undefined` (e.g. when cert files come from env vars only). This is inconsistent with the federated credential case below, which uses `if (creds.ficClientId)` guards.

If `creds.certPemFile` is `undefined` (because the file path was set via `process.env.certPemFile` rather than typed config), `base.certPemFile` is explicitly set to `undefined`. Depending on how `getAuthConfigWithDefaults` inspects its argument — e.g. `'certPemFile' in config` vs `config.certPemFile` — the SDK's own env var resolution may be bypassed, leaving the adapter without a cert path at runtime.

The federated case correctly avoids this:
```ts
if (creds.ficClientId) base.FICClientId = creds.ficClientId;
if (creds.widAssertionFile) base.WIDAssertionFile = creds.widAssertionFile;
```

Consider aligning the certificate case:
```suggestion
    case "certificate":
      if (creds.certPemFile) base.certPemFile = creds.certPemFile;
      if (creds.certKeyFile) base.certKeyFile = creds.certKeyFile;
      if (creds.sendX5C != null) base.sendX5C = creds.sendX5C;
      break;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/msteams/src/token.test.ts
Line: 69-84

Comment:
**Test only asserts `authType`, not resolved cert file paths**

The test verifies that `hasCertificateAuthMaterial` passes (so `authType` is returned), but it never asserts that `certPemFile`/`certKeyFile` are correctly propagated when the values come from env vars exclusively. Since `resolveMSTeamsCredentials` currently assigns `certPemFile: cfg?.certPemFile` (which is `undefined` when only the env var is set), a consuming component that relies on `creds.certPemFile` being non-null would see `undefined`. Adding assertions like:

```ts
expect(resolved?.certPemFile).toBeUndefined(); // explicitly documenting env-only sourcing
// or, if the intent is for the SDK to handle the env var:
expect(resolved?.certPemFile).toBe("/env/cert.pem"); // if paths should be forwarded
```

…would clarify the intended contract and catch regressions if the behavior changes.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: 703377a}

CI failures in this run are pre-existing issues on main, not related to this PR's changes:

1. tsgo type error — normalizeAgentId not found in src/gateway/hooks.ts:276,349. This PR does not touch src/gateway/hooks.ts or any agent/gateway code.
2. Startup memory check — status --json used 930 MB RSS vs 900 MB limit. This is a flaky resource constraint on the CI runner, unrelated to msteams extension changes.

Both failures reproduce on current main without this PR's commits. The msteams test suite passes locally (244/244 tests, pnpm build && pnpm check green except for the same pre-existing normalizeAgentId error).