Skip to content

Persist SPFxScaffoldLog to disk and restrict --package-manager to new projects#221

Merged
nick-pape merged 5 commits intoSharePoint:mainfrom
nick-pape:claude/audit-log-package-manager-165
Apr 2, 2026
Merged

Persist SPFxScaffoldLog to disk and restrict --package-manager to new projects#221
nick-pape merged 5 commits intoSharePoint:mainfrom
nick-pape:claude/audit-log-package-manager-165

Conversation

@nick-pape
Copy link
Copy Markdown
Contributor

@nick-pape nick-pape commented Apr 1, 2026

Description

Closes #165 (restrict --package-manager to new projects). Partially addresses #100 (scaffold log persistence).

  • Adds loadAsync, saveAsync, and hasEntries to SPFxScaffoldLog so it can be persisted as .spfx-scaffold.jsonl in the project root
  • CreateAction loads the log from disk to detect existing projects. When --package-manager is specified on an existing project, it emits a warning and skips installation
  • The log file accumulates events (JSONL) across runs, recording templates rendered, files written/merged, and package manager usage

How was this tested?

  • rush build passes for both @microsoft/spfx-template-api and @microsoft/spfx-cli
  • 10 new unit tests for SPFxScaffoldLog persistence (hasEntries, loadAsync, saveAsync, round-trip)
  • 8 new unit tests for CreateAction (existing project: warn + skip install; new project: allow install + save log)
  • All existing tests continue to pass (314 API + 105 CLI)

Type of change

  • New feature / enhancement

🤖 Generated with Claude Code

@nick-pape nick-pape force-pushed the claude/audit-log-package-manager-165 branch from 0613a0e to 8efd8b4 Compare April 1, 2026 19:30
@nick-pape nick-pape changed the title Implement SPFxCreationAuditLog and restrict --package-manager to new projects Persist SPFxScaffoldLog to disk and restrict --package-manager to new projects Apr 1, 2026
@nick-pape nick-pape force-pushed the claude/audit-log-package-manager-165 branch 2 times, most recently from 463d91b to dd85453 Compare April 1, 2026 20:06
… projects

Adds loadAsync/saveAsync/hasEntries to SPFxScaffoldLog so it can be persisted
as .spfx-scaffold.jsonl. CreateAction now loads the log from disk to detect
existing projects and ignores --package-manager with a warning when adding
components to an existing solution.

Closes SharePoint#100, closes SharePoint#165.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@nick-pape nick-pape force-pushed the claude/audit-log-package-manager-165 branch from dd85453 to 0e376da Compare April 1, 2026 20:26
@nick-pape
Copy link
Copy Markdown
Contributor Author

Manual test results

All scenarios tested locally with SPFX_CI_MODE=1 and --local-source templates to avoid network calls.

# Scenario Expected Result
1 New project, --package-manager omitted Scaffold only, no install, .spfx-scaffold.jsonl created ✅ Pass
2 New project, --package-manager npm Scaffold + npm install, log records npm ✅ Pass
3 Existing project (npm in log), --package-manager npm No warning, runs npm install ✅ Pass
4 Existing project (npm in log), --package-manager pnpm Warning emitted, overrides to npm install ✅ Pass
5 Existing project (npm in log), --package-manager none No warning, no install ✅ Pass
6 Existing project (npm in log), --package-manager omitted No warning, no install ✅ Pass

Additional checks

  • ✅ Coding standards reviewed against CLAUDE.md — all compliant (one pre-existing JSON.parse/JSON.stringify in JSONL serialization, not introduced by this PR)
  • ✅ Unit tests: 319 API + 105 CLI, all passing
  • .spfx-scaffold.jsonl excluded from integration test file comparison
  • lastPackageManager reads from log before appending the current run's event (bug caught during manual testing, fixed)

@nick-pape nick-pape marked this pull request as ready for review April 1, 2026 20:36
Copilot AI review requested due to automatic review settings April 1, 2026 20:36
@nick-pape nick-pape enabled auto-merge (squash) April 1, 2026 20:37
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces persistence for SPFxScaffoldLog (as .spfx-scaffold.jsonl in the project root) and uses that persisted log in spfx create to detect existing projects and restrict how --package-manager behaves.

Changes:

  • Add hasEntries, lastPackageManager, loadAsync, and saveAsync to SPFxScaffoldLog and export SCAFFOLD_LOG_FILENAME.
  • Update CreateAction to load/save the scaffold log and adjust package-manager behavior for existing projects.
  • Add/adjust unit tests, snapshots, and documentation to account for the new log file and CLI behavior.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
tests/spfx-template-test/src/tests/templates.test.ts Ignore .spfx-scaffold.jsonl in scaffolded output comparisons.
common/docs/spfx-cli-architecture.md Document scaffold log persistence and describe existing-project behavior.
common/changes/@microsoft/spfx-cli/audit-log-package-manager-restriction_2026-04-01.json Rush change file for lockstep policy.
apps/spfx-cli/src/cli/actions/tests/CreateAction.test.ts Add tests covering existing-project package-manager behavior and log saving.
apps/spfx-cli/src/cli/actions/tests/snapshots/CreateAction.test.ts.snap Snapshot updates for new tests/output.
apps/spfx-cli/src/cli/actions/CreateAction.ts Load persisted scaffold log, use it for “existing project” detection, and save it after runs.
apps/spfx-cli/README.md Document --package-manager and its behavior for existing projects.
api/spfx-template-api/src/logging/test/SPFxScaffoldLog.test.ts Add unit tests for persistence helpers and new getters.
api/spfx-template-api/src/logging/SPFxScaffoldLog.ts Implement log persistence and new API surface.
api/spfx-template-api/src/logging/index.ts Export SCAFFOLD_LOG_FILENAME.
api/spfx-template-api/src/index.ts Re-export SCAFFOLD_LOG_FILENAME from the package root.
api/spfx-template-api/README.md Update usage examples to show loadAsync/saveAsync.
api/spfx-template-api/etc/spfx-template-api.api.md API report updates for new exports/members.
Comments suppressed due to low confidence (1)

apps/spfx-cli/src/cli/actions/CreateAction.ts:242

  • package-manager-selected is appended before resolving the effective package manager for existing projects. If --package-manager gets overridden later, the persisted scaffold log will record the requested value (and become the new lastPackageManager), which can flip the log’s package-manager history and break subsequent runs. Append the event after resolving, or record the resolved value (and optionally add a separate field/event for the user-requested value).
      log.append({
        kind: 'package-manager-selected',
        packageManager,
        targetDir
      });

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts
Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts Outdated
Comment thread api/spfx-template-api/src/logging/SPFxScaffoldLog.ts Outdated
Comment thread api/spfx-template-api/src/logging/SPFxScaffoldLog.ts Outdated
Comment thread api/spfx-template-api/README.md Outdated
Comment thread common/docs/spfx-cli-architecture.md Outdated
Comment thread common/docs/spfx-cli-architecture.md Outdated
Comment thread apps/spfx-cli/README.md Outdated
- Validate previousPackageManager against known values before cast
- Fix README example referencing deleted SPFxCreationAuditLog
- Update all docs to describe override-and-warn behavior (not "ignored")

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread api/spfx-template-api/etc/spfx-template-api.api.md Outdated
Comment thread api/spfx-template-api/src/logging/SPFxScaffoldLog.ts
Comment thread api/spfx-template-api/src/logging/SPFxScaffoldLog.ts Outdated
Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts Outdated
Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts Outdated
Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts
- Rename loadAsync/saveAsync to loadFromFolderAsync/saveToFolderAsync
- Cache lastPackageManager in a field, update on append() (O(1) reads)
- Simplify catch clause: remove `as Error` cast for isNotExistError
- Add ISessionStartedEvent, log session-started at top of CreateAction
- Extract VALID_PACKAGE_MANAGERS to module-level ReadonlySet<PackageManager>
- Improve warning message: extract longName, append "will be ignored"
- Update READMEs, docs, tests, and API report

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 2, 2026 03:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread api/spfx-template-api/src/logging/SPFxScaffoldLog.ts
Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts
Comment thread apps/spfx-cli/src/cli/actions/tests/CreateAction.test.ts
@nick-pape
Copy link
Copy Markdown
Contributor Author

Manual Test Pass (post-review, commit 45563b9)

Re-ran the full 6-scenario matrix after addressing Ian's review feedback. All pass.

# Scenario Expected Result
1 New project, no --package-manager Scaffold only, log created with session-started first
2 New project, --package-manager npm Scaffold + npm install, package-manager-install-completed logged
3 Existing project (npm in log), --package-manager npm No warning, npm install runs
4 Existing project (npm in log), --package-manager pnpm Warning with improved message ("will be ignored"), overrides to npm
5 Existing project (npm in log), --package-manager none No warning, no install
6 Existing project (npm in log), omitted No warning, no install

Additional checks:

  • session-started event is the first event in every log entry (verified scenario 1 log)
  • 5 session-started events accumulated in scenario 2's log across runs 2-6
  • lastPackageManager cache works correctly (override triggers on scenario 4 but not 3)
  • Warning message now reads: --package-manager "pnpm" is overridden by "npm" from the existing project's scaffold log. The --package-manager parameter will be ignored.

Review changes addressed in this round:

  • loadAsyncloadFromFolderAsync, saveAsyncsaveToFolderAsync
  • lastPackageManager cached in _lastPackageManager field (O(1), prepopulated via append())
  • Simplified catch (error) clause (removed as Error cast)
  • Added ISessionStartedEvent logged at top of each spfx create invocation
  • Extracted VALID_PACKAGE_MANAGERS: ReadonlySet<PackageManager> to module level
  • Improved warning message with extracted longName and "will be ignored" suffix

- Move package-manager-selected log append to after override resolution
  so the log records the *resolved* PM, not the user-provided one
- Only log package-manager-selected when actually installing (not none)
  so skipping install doesn't erase the previously recorded PM
- Make lastPackageManager sticky: "none" no longer clears the cache
- Add test assertions verifying what gets appended to the scaffold log

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
iclanton
iclanton previously approved these changes Apr 2, 2026
Comment thread api/spfx-template-api/src/logging/test/SPFxScaffoldLog.test.ts
Comment thread api/spfx-template-api/src/logging/test/SPFxScaffoldLog.test.ts
Comment thread api/spfx-template-api/src/logging/test/SPFxScaffoldLog.test.ts Outdated
Comment thread api/spfx-template-api/src/logging/test/SPFxScaffoldLog.test.ts Outdated
Comment thread api/spfx-template-api/src/logging/SPFxScaffoldLog.ts
- saveToFolderAsync and round-trip test use .name pattern
- hasEntries/lastPackageManager kept as strings (getters lack .name)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 2, 2026 20:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts
Comment thread apps/spfx-cli/src/cli/actions/CreateAction.ts
@nick-pape nick-pape merged commit 04cbc21 into SharePoint:main Apr 2, 2026
8 checks passed
iclanton pushed a commit to iclanton/spfx that referenced this pull request Apr 2, 2026
… projects (SharePoint#221)

## Description

Closes SharePoint#165 (restrict `--package-manager` to new projects). Partially
addresses SharePoint#100 (scaffold log persistence).

- Adds `loadAsync`, `saveAsync`, and `hasEntries` to `SPFxScaffoldLog`
so it can be persisted as `.spfx-scaffold.jsonl` in the project root
- `CreateAction` loads the log from disk to detect existing projects.
When `--package-manager` is specified on an existing project, it emits a
warning and skips installation
- The log file accumulates events (JSONL) across runs, recording
templates rendered, files written/merged, and package manager usage

## How was this tested?

- `rush build` passes for both `@microsoft/spfx-template-api` and
`@microsoft/spfx-cli`
- 10 new unit tests for `SPFxScaffoldLog` persistence (`hasEntries`,
`loadAsync`, `saveAsync`, round-trip)
- 8 new unit tests for `CreateAction` (existing project: warn + skip
install; new project: allow install + save log)
- All existing tests continue to pass (314 API + 105 CLI)

## Type of change
- [x] New feature / enhancement

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
iclanton added a commit to iclanton/spfx that referenced this pull request Apr 2, 2026
Follow-up to SharePoint#221: since scaffold now persists the log to disk, gitignore
it in every template, example, and test fixture so it doesn't appear as
an untracked file in generated projects.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
iclanton added a commit to iclanton/spfx that referenced this pull request Apr 2, 2026
Follow-up to SharePoint#221: since scaffold now persists the log to disk, gitignore
it in every template, example, and test fixture so it doesn't appear as
an untracked file in generated projects.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
iclanton added a commit that referenced this pull request Apr 6, 2026
…227)

## Description

Follow-up to #221: since the scaffold now persists the log to disk as
`.spfx-scaffold.jsonl`, this PR gitignores that file in every template,
example, and test fixture so it doesn't appear as an untracked file in
generated projects.

## How was this tested

- `rushx build` in `tests/spfx-template-test` — snapshot updated to
reflect that templates now emit a `.gitignore` file (file count 1→2)

## Type of change

- [ ] Bug fix
- [ ] New feature
- [x] Template change (modifies template files or examples)
- [ ] Docs / CI change only

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
iclanton added a commit that referenced this pull request Apr 6, 2026
…227)

## Description

Follow-up to #221: since the scaffold now persists the log to disk as
`.spfx-scaffold.jsonl`, this PR gitignores that file in every template,
example, and test fixture so it doesn't appear as an untracked file in
generated projects.

## How was this tested

- `rushx build` in `tests/spfx-template-test` — snapshot updated to
reflect that templates now emit a `.gitignore` file (file count 1→2)

## Type of change

- [ ] Bug fix
- [ ] New feature
- [x] Template change (modifies template files or examples)
- [ ] Docs / CI change only

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Restrict --package-manager to new projects once SPFxCreationAuditLog is implemented

3 participants