Add SPFxScaffoldLog structured event log#206
Add SPFxScaffoldLog structured event log#206nick-pape wants to merge 16 commits intoSharePoint:mainfrom
Conversation
Introduces a typed, JSONL-serializable event log that records key decisions and outcomes during scaffolding. This is the foundation for the audit log file described in SharePoint#100. New logging module in spfx-template-api: - SPFxScaffoldEvent discriminated union (template-rendered, package-manager-selected, file-write, package-manager-install-completed) - SPFxScaffoldLog class with append, getEventsByKind, toJsonl/fromJsonl SPFxTemplateWriter now accepts an optional log and emits a file-write event per file with outcome (new/merged/preserved/unchanged). CreateAction creates a log and appends template-rendered, package-manager-selected, and install-completed events. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new structured scaffolding event log to @microsoft/spfx-template-api and wires it into the CLI scaffolding flow to record template render decisions, per-file write outcomes, and package manager actions (as groundwork for the audit log in #100).
Changes:
- Introduces
loggingmodule withSPFxScaffoldEvent(discriminated union) andSPFxScaffoldLog(append/filter + JSONL serialize/deserialize). - Extends
SPFxTemplateWriter.writeAsync()with optional{ log }to emitfile-writeevents (new/merged/preserved/unchanged). - Integrates log creation + event emission into
spfx create(template-rendered, package-manager-selected, package-manager-install-completed).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| common/changes/@microsoft/spfx-cli/scaffold-log_2026-03-27.json | Adds Rush change file for the lockstep policy. |
| apps/spfx-cli/src/cli/actions/CreateAction.ts | Creates/uses SPFxScaffoldLog, emits scaffold events, and threads log into the writer. |
| api/spfx-template-api/src/writing/SPFxTemplateWriter.ts | Adds IWriteOptions + optional log emission for per-file outcomes. |
| api/spfx-template-api/src/writing/index.ts | Re-exports IWriteOptions from the writing barrel. |
| api/spfx-template-api/src/writing/test/SPFxTemplateWriter.test.ts | Adds integration tests validating file-write events across outcomes and backward compatibility. |
| api/spfx-template-api/src/logging/SPFxScaffoldEvent.ts | Defines the scaffold event union and related event interfaces/types. |
| api/spfx-template-api/src/logging/SPFxScaffoldLog.ts | Implements append-only in-memory log + JSONL (de)serialization and kind filtering. |
| api/spfx-template-api/src/logging/index.ts | Adds logging barrel exports. |
| api/spfx-template-api/src/logging/test/SPFxScaffoldLog.test.ts | Adds unit tests for append/filtering/JSONL round-trips and timestamp autofill. |
| api/spfx-template-api/src/index.ts | Exports new logging types and SPFxScaffoldLog from the package root. |
| api/spfx-template-api/etc/spfx-template-api.api.md | Updates API extractor output for the new public types and updated writer signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Manual Test ReportEnvironment
1. Automated Tests
Full 2. SPFxScaffoldLog Unit Behavior (programmatic)
3. Writer + Log Integration — Fresh ScaffoldScaffolded
4. Writer + Log Integration — Merge Scenario (Add Component)Scaffolded
5. CLI End-to-End
Both runs completed without errors. The log is created and populated in-memory (not persisted to disk yet — that's #100). 6. Backward Compatibility
|
- Return defensive copy from SPFxScaffoldLog.events getter - Use append() in fromJsonl() so timestamps are normalized - Narrow readFile catch to ENOENT only, rethrow other errors - Record package-manager-selected unconditionally (including 'none') - Restructure install flow so completion event is logged before throw - Add test for non-ENOENT readFile error propagation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ent-log # Conflicts: # api/spfx-template-api/etc/spfx-template-api.api.md # api/spfx-template-api/src/index.ts # apps/spfx-cli/src/cli/actions/CreateAction.ts
- Rename getEventsByKind → getEventsOfKind - Use @remarks JSDoc tag for mergeHelper field - Add signal field to IPackageManagerInstallCompletedEvent - Make timestamp optional on append() via SPFxScaffoldEventInput type - Doc comment: "defensive shallow copy" - Optimize fromJsonl to single-pass character scanning - Add snapshot assertions to writer log integration tests - Extract _logFileWrite to loose function, use log?.append() - Simplify mergeHelper serialization (JSON.stringify drops undefined) - Remove redundant timestamp: '' from CLI event appends Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Manual Test Report (v2 — post review fixes)Re-run after addressing all review feedback (Copilot + iclanton) and merging from main (CasedString PR #194, cwd mock PR #204, CRLF PR #208). Environment
1. Automated Tests
2. SPFxScaffoldLog Unit Behavior (26 programmatic assertions)
3. Writer + Log — Fresh ScaffoldScaffolded
4. Writer + Log — Merge ScenarioScaffolded
5. Backward Compatibility
6. CLI End-to-End
7. Review Changes Verified
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Inline distributive Omit to avoid _DistributiveOmit leaking into API - Handle \r\n line endings in fromJsonl for cross-platform support - Only include mergeHelper field on file-write events when defined Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The _logFileWrite fix correctly omits mergeHelper when undefined, so the snapshots no longer contain "mergeHelper": undefined entries. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ent-log Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Enable resolveJsonModule in spfx-cli tsconfig and replace the require() call with a typed import, as suggested by iclanton. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- fromJsonl: use indexOf('\n') instead of character-by-character scanning
- _logFileWrite: simplify to just log?.append({...}) — no elaborate inline type
- SPFxTemplateWriter: use FileSystem.isNotExistError instead of manual ENOENT check
- IWriteOptions.log doc: clarify it covers non-deleted entries with non-null contents
- CreateAction: fix CRLF line endings to LF
- CreateAction: use IWaitForExitResultWithoutOutput from Executable.waitForExitAsync
- CreateAction: move log append + error handling into _runInstallAsync
- CreateAction: move package-manager-selected event before writeAsync
- resolveJsonModule: move from spfx-cli tsconfig to the rig
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…r package.json The resolveJsonModule setting was added to the shared rig tsconfig in a previous review round, but it should be handled by a separate PR. Revert CLI_VERSION back to require() since the rig no longer enables resolveJsonModule. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
api/spfx-template-api/src/writing/test/SPFxTemplateWriter.test.ts
Outdated
Show resolved
Hide resolved
…ent-log # Conflicts: # apps/spfx-cli/src/cli/actions/CreateAction.ts
- Switch require() to import for package.json in CreateAction (iclanton) - Replace delete with destructuring in snapshotLog test helper (iclanton) - Remove unnecessary spread in SPFxScaffoldLog.append, mutate timestamp directly (iclanton) - Fix IFileWriteEvent doc to clarify deleted/null entries are skipped (copilot) - Fix enoent() test helper to include errno/syscall for FileSystem.isNotExistError compat - Update snapshots Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ent-log Integrates PR SharePoint#207 (Remove mem-fs-editor) with scaffold event log. Adapts logging to the new TemplateOutput-based SPFxTemplateWriter API. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Resolve merge conflicts from SharePoint#212 (CRLF line endings fix) - Fix append() to shallow-clone events instead of mutating caller's object - Normalize exitCode consistently in _runInstallAsync using a single variable - Clarify FileWriteOutcome 'new' docs to cover binary file overwrite case Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Manual Test Report — Post-merge with PR #207 (Remove mem-fs-editor)Branch: Test 1: Unit Test Suites
Test 2: End-to-End ScaffoldingRan the CLI with Result: PASSED Test 3: SPFxScaffoldLog API
Result: PASSED Test 4:
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (existingBuffer === undefined) { | ||
| contentToWrite = contents; | ||
| _logFileWrite(log, relativePath, 'new'); | ||
| } else if (!existingBuffer.equals(contents)) { | ||
| contentToWrite = contents; |
There was a problem hiding this comment.
_logFileWrite() is called before writeFileAsync() executes. If the subsequent write fails (e.g., EACCES/disk full), the log will still record outcomes like new/merged even though the file was not actually written, which makes the audit trail misleading. Consider appending file-write events only after a successful write, and/or introducing a failure outcome/event when writeFileAsync() throws.
| * When provided, a `file-write` event is appended for each non-deleted file with | ||
| * non-null contents processed during the write phase. |
There was a problem hiding this comment.
The IWriteOptions.log doc refers to “non-deleted” files and “non-null contents”, but TemplateOutput/ITemplateOutputEntry don’t represent deletions and contents can’t be null. This wording is misleading for API consumers; consider rephrasing to simply describe that a file-write event is recorded for each file in templateOutput.files that is evaluated for writing/merge.
| * When provided, a `file-write` event is appended for each non-deleted file with | |
| * non-null contents processed during the write phase. | |
| * When provided, a `file-write` event is appended for each file in the template | |
| * output that is evaluated for writing or merging during the write phase. |
| export interface IPackageManagerSelectedEvent extends ISPFxScaffoldEventBase { | ||
| kind: 'package-manager-selected'; | ||
| packageManager: string; | ||
| targetDir: string; |
There was a problem hiding this comment.
IPackageManagerSelectedEvent.targetDir is currently an arbitrary string and in CLI usage it’s an absolute path. If this log is later persisted/committed (per #100’s audit-log goals), absolute paths can leak PII (e.g., usernames/home directories) and reduce portability. Consider storing a path relative to the project root (or just the folder name), or omitting/redacting this field from the public event schema.
| targetDir: string; | |
| /** | |
| * Target directory for the package manager operation, expressed as a path | |
| * relative to the project root (for example, ".", | |
| * "src", or "packages/my-project"). | |
| * | |
| * Absolute paths MUST NOT be used here to avoid leaking PII (such as user | |
| * home directories) into persisted logs and to improve portability. | |
| */ | |
| targetDirRelativePath: string; |
There was a problem hiding this comment.
We aren't sending these logs anywhere, so PII isn't a concern.
|
|
||
| // ---- getEventsOfKind --------------------------------------------------- | ||
|
|
||
| describe('getEventsOfKind', () => { |
There was a problem hiding this comment.
| describe('getEventsOfKind', () => { | |
| describe(SPFxScaffoldLog.prototype.getEventsOfKind.name, () => { |
|
|
||
| // ---- JSONL serialization ----------------------------------------------- | ||
|
|
||
| describe('toJsonl', () => { |
There was a problem hiding this comment.
| describe('toJsonl', () => { | |
| describe(SPFxScaffoldLog.prototype.toJsonl.name, () => { |
| }); | ||
| }); | ||
|
|
||
| describe('fromJsonl', () => { |
There was a problem hiding this comment.
| describe('fromJsonl', () => { | |
| describe(SPFxScaffoldLog.fromJsonl.name, () => { |
| export interface IPackageManagerSelectedEvent extends ISPFxScaffoldEventBase { | ||
| kind: 'package-manager-selected'; | ||
| packageManager: string; | ||
| targetDir: string; |
There was a problem hiding this comment.
We aren't sending these logs anywhere, so PII isn't a concern.
| * | ||
| * @public | ||
| */ | ||
| export type SPFxScaffoldEvent = |
There was a problem hiding this comment.
| export type SPFxScaffoldEvent = | |
| export type ISPFxScaffoldEvent = |
| * | ||
| * @public | ||
| */ | ||
| export type SPFxScaffoldEventInput = SPFxScaffoldEvent extends infer E |
There was a problem hiding this comment.
| export type SPFxScaffoldEventInput = SPFxScaffoldEvent extends infer E | |
| export type ISPFxScaffoldEventInput = SPFxScaffoldEvent extends infer E |
| const normalizedEvent: SPFxScaffoldEvent = { | ||
| ...event, | ||
| timestamp: event.timestamp || new Date().toISOString() | ||
| } as SPFxScaffoldEvent; |
There was a problem hiding this comment.
Why do we need this cast? Shouldn't adding timestamp make the typechecker happy?
Description
Introduces a typed, JSONL-serializable event log (
SPFxScaffoldLog) that records key decisions and outcomes during scaffolding. This is the foundation for the audit log file described in #100.New
loggingmodule in@microsoft/spfx-template-api:SPFxScaffoldEvent— discriminated union of four event kinds:template-rendered,package-manager-selected,file-write,package-manager-install-completedSPFxScaffoldLog— append-only log class withgetEventsOfKind()filtering andtoJsonl()/fromJsonl()serializationSPFxTemplateWriterintegration:writeAsync()now accepts an optional{ log }parameterfile-writeevent per non-deleted file with non-null contents, recording outcome (new/merged/preserved/unchanged)CLI integration (
CreateAction):SPFxScaffoldLogat the start of scaffoldingtemplate-rendered(with full render context),package-manager-selected, andpackage-manager-install-completedeventsThe log is created and populated in-memory but not yet persisted to disk — that will be handled by #100.
How was this tested?
SPFxScaffoldLog(append/read, filtering, JSONL round-trips, timestamp auto-fill, edge cases)SPFxTemplateWriterlog events (one per file outcome + multi-file + backward-compat + skip deleted)rush buildType of change