Math.random().toString(36).slice(2,6) can produce fewer than 4 chars
for short base-36 representations (e.g. 0.5 → '0.i' → 'i').
Append '0000' before slicing to guarantee the suffix is always 4 chars.
The test drainPostHookActions helper iterated the live array, but
triggerInternalHook snapshots with [...(event.postHookActions ?? [])]
before draining. Align test helper to match production semantics.
1. Math.random() === 0 guard: (0).toString(36).slice(2,6) returns ''
producing a trailing-hyphen slug. Added || '0000' fallback.
2. Misleading ENOENT comment: said 'when blockSessionSave was set before
writeFileWithinRoot' but that branch requires writtenEntry !== null
(blockSessionSave was false). Real scenario: external file deletion
between inline write and post-hook drain. Comment corrected.
3. Live-array drain: for...of on postHookActions is a live iterator —
a self-scheduling action could loop infinitely. Snapshot the array
with [...(event.postHookActions ?? [])] before draining.
Split the single catch block into two distinct error-handling paths:
1. Pre-existing content restore (writeFileWithinRoot): errors are NOT
swallowed — ENOENT here means memoryDir was removed after our inline
write, which is a real filesystem inconsistency that must surface.
2. Unlink (no pre-existing content): ENOENT is expected when the inline
write didn't happen. Non-ENOENT errors (EACCES, EROFS) are re-thrown
for triggerInternalHook to log. Added comment clarifying that the
retraction guarantee is best-effort under adversarial filesystem
conditions (per-action isolation absorbs the error).
When blockSessionSave is pre-set, the handler skips transcript loading
and LLM slug generation (avoiding unnecessary I/O and model calls).
If a later hook clears blockSessionSave without setting sessionSaveContent,
no file can be produced because the transcript was never loaded.
Previously this was a silent no-op. Now emits log.warn so plugin authors
know to supply sessionSaveContent when un-blocking a pre-set block.
Adds test: 'blockSessionSave pre-set then cleared without sessionSaveContent
warns and writes nothing' — verifies the edge case produces no file.
When a later hook sets blockSessionSave=true, the retraction previously
unconditionally deleted memoryFilePath. If the filename collided with a
pre-existing memory file (e.g. same LLM slug on the same day), the prior
session's content was erased.
Now snapshots any pre-existing file content before the inline write and
restores it on retraction instead of deleting. Non-colliding writes still
get deleted as before.
The resolveDisplaySessionKey function correctly resolved workspace-based
agent IDs (e.g. agent:main:main → agent:navi:main when workspaceDir
matches the navi agent), but the entry template used event.sessionKey
directly, bypassing the resolution.
When an earlier hook pre-sets sessionSaveContent and a later hook clears
it, the transcript is not available for fallback — it was never loaded.
Hooks wanting to override should set their own content, not clear it.
The InternalHookEvent interface now requires postHookActions (added by
this PR). The boot-md test's makeEvent helper was missing it, causing
a tsc error that fails CI.
Second-resolution (HHMMSS) fallback slugs can collide when automated or
multi-channel setups emit rapid /new or /reset commands within the same
second — both writes target the same filename and the later one silently
overwrites the earlier memory entry.
Append a 4-char random alphanumeric suffix (e.g. 103022-x7f2) to make
collisions effectively impossible without LLM slug generation.
The slug generator embeds up to 2000 chars of raw conversation content
in its prompt. Without disableTools, the embedded agent inherits the
full tool set (exec, file write, messaging), meaning a crafted
conversation could prompt-inject the slug call into executing arbitrary
side-effects before the slug text is extracted.
Slug generation is pure text — it never needs tool access. Add
disableTools: true to close this injection surface.
- Remove redundant length guard on postHookActions drain loop
- Add per-action error isolation to drainPostHookActions test helper,
matching actual triggerInternalHook behaviour
- Remove double-catch in session-memory post-hook callback; let errors
propagate to the framework's per-action catch for consistent logging
Remove early return in triggerInternalHook that skipped the post-hook
drain when allHandlers was empty. This was a latent footgun: any
postHookActions pre-populated on the event before calling
triggerInternalHook would be silently dropped if no handlers were
registered for that event type.
Update test to verify post-hooks now drain in the no-handlers case.
When blockSessionSave is true initially, the inline write is skipped —
including the fs.mkdir that creates memoryDir. If a later hook clears
blockSessionSave and sets sessionSaveContent, the post-hook
writeFileWithinRoot call would fail with ENOENT because the directory
was never created. The error was silently swallowed, causing the content
override to be lost.
Add fs.mkdir(memoryDir, { recursive: true }) before the post-hook write.
Add regression test for the block-then-clear-with-content scenario.
When blockSessionSave is already true, the handler was still loading
session content and potentially calling generateSlugViaLLM — sending
sensitive transcript text to a model provider despite an explicit
block. Short-circuit before transcript loading when the flag is set.
Credit: Codex review.
When both flags are set, blockSessionSave must win — a blocked save
should never create a file, even if sessionSaveContent is also present.
Bug: if blockSessionSave was pre-set (writtenEntry=null), the post-hook
sessionSaveContent check would pass and create a new file, violating
the block intent.
Fix: guard the sessionSaveContent overwrite with blockSessionSave !== true.
Tests: 2 new tests covering both-flags-pre-set and both-flags-late-set.
Credit: Greptile review.
Adds a generic postHookActions mechanism to InternalHookEvent:
handlers push deferred callbacks that triggerInternalHook drains
after all handlers complete. This eliminates FIFO registration-order
dependencies — any hook can set context flags regardless of when it
registered.
Session-memory handler updated to use fail-safe pattern:
- Writes file inline (preserves data if postHookActions never runs)
- Pushes post-hook action for retraction (blockSessionSave) and
content replacement (sessionSaveContent)
- Pre-set flags still work inline; late-set flags work via post-hook
New tests:
- 5 tests for postHookActions mechanism (ordering, error isolation,
late-context visibility)
- 2 tests for late-set blockSessionSave/sessionSaveContent
- 1 fail-safe test (file preserved when postHookActions not drained)
Custom content and no-session paths fell through to HHMM (4-char)
timestamp slug. Two /new events in the same minute would overwrite.
HHMMSS makes collisions require same-second timing.
Bundled hooks register before managed/workspace hooks in FIFO order,
so blockSessionSave only works when set by typed plugin hooks (which
fire earlier in the lifecycle) or extraDirs hooks. Document this
limitation honestly rather than claiming incorrect ordering.
- Add log.debug when sessionSaveContent override is used (symmetric with blockSessionSave)
- Change override test assertion from toContain to toBe for precision
Two new context fields for upstream hooks (e.g. security plugins) to
control session memory persistence:
- blockSessionSave (boolean): prevent session from being saved to memory
- sessionSaveContent (string): override saved content with custom text
(empty string is valid — persists a blank marker without transcript)
When sessionSaveContent is set, LLM slug generation and session content
loading are skipped (unnecessary when content is overridden).
Split from #35567 — sessionSaveRedirectPath follows separately as it
requires path canonicalization, symlink resolution, and filesystem
write policy review.
* build: mirror uuid for msteams
Add uuid to both the msteams bundled extension and the root package so the workspace build can resolve @microsoft/agents-hosting during tsdown while standalone extension installs also have the runtime dependency available.
Regeneration-Prompt: |
pnpm build failed because @microsoft/agents-hosting 1.3.1 requires uuid in its published JS but does not declare it in its package manifest. The msteams extension dynamically imports that package, and the workspace build resolves it from the root dependency graph. Mirror uuid into the root package for workspace builds and keep it in extensions/msteams/package.json so standalone plugin installs also resolve it. Update the lockfile to match the manifest changes.
* build: prune stale plugin dist symlinks
Remove stale dist and dist-runtime plugin node_modules symlinks before tsdown runs. These links point back into extension installs, and tsdown's clean step can traverse them on rebuilds and hollow out the active pnpm dependency tree before plugin-sdk declaration generation runs.
Regeneration-Prompt: |
pnpm build was intermittently failing in the plugin-sdk:dts phase after earlier build steps had already run. The symptom looked like missing root packages such as zod, ajv, commander, and undici even though a fresh install briefly fixed the problem. Investigate the build pipeline step by step rather than patching TypeScript errors. Confirm whether rebuilds mutate node_modules, identify the first step that does it, and preserve existing runtime-postbuild behavior.
The key constraint is that dist and dist-runtime plugin node_modules links are intentional for runtime packaging, so do not remove that feature globally. Instead, make rebuilds safe by deleting only stale symlinks left in generated output before invoking tsdown, so tsdown cleanup cannot recurse back into the live pnpm install tree. Verify with repeated pnpm build runs.
