feat(mcp): set and clear preferred settings sets (PP-u4ab.26)

[timothyfroehlich]

Work bead PP-u4ab.26 (preferred-set half only).

Changes

• src/lib/mcp/tools/update-settings-set.ts: added optional preferredHouse and preferredTournament booleans.
• Calls setPreferredSettingsSet({ setId, actor, slot, preferred }) in src/services/machine-settings.ts.
• Pre-validates up front against canManageMachineSettings and verifies the set carries (or will carry) the required slot tag before writing.
• Tests: added tests to the settings sets describe block in src/test/integration/mcp-tools.test.ts verifying technician/owner/admin rights, member denial, missing-tag refusal, clearing existing preferred sets, and timeline events (settings_preferred_changed).
• Docs: updated plugins/pinpoint/skills/pinpoint-mcp/SKILL.md, references/tools.md, and references/settings-sets.md to document preferred sets, note PERMANENT class for preferred changes, and state that making a set preferred makes it a community set.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
pin-point	Ready	Preview	Oct 6, 2026 11:33pm UTC

[timothyfroehlich]

Review of 28f4ab4 (level: medium)

The tool calls setPreferredSettingsSet rather than reimplementing it (CORE-ARCH-014), the untagged-set pre-check is real (mutation-checked: removing it makes the "writes nothing even with content edits" test fail), and the tests cover tech/owner/admin success, clearing the previous holder, the community flip and the settings_preferred_changed event. Three findings:

1. High: partial write when clearing preferred and removing the tag in one call (src/lib/mcp/tools/update-settings-set.ts:179)
The tag pre-check now evaluates house: false against the requested preferred state, so { house: false, preferredHouse: false } on the preferred House set passes pre-validation. Execution order is content, then tags (line 263), then preferred (line 281). setSettingsSetTag still sees the set as preferred and refuses ("Unset the preferred House set before removing its tag."), but by then updateSettingsSet has already committed. An integration probe with name: "Renamed", house: false, preferredHouse: false errored and left the name Renamed plus a settings_set_updated event, with the tag and preferred flag unchanged. That is the half-landed call the brief rules out. Fix: apply preferred changes between tag adds and tag removals (adds, then preferred, then removals), or refuse the combination up front, and add a test for it.

2. Medium: the docs say preferred changes notify people, and they don't (plugins/pinpoint/skills/pinpoint-mcp/SKILL.md:45, references/tools.md:149)
setPreferredSettingsSet only writes timeline events. docs/feature-specs/machine-settings.md records spec 4.6 as a known divergence: "Changing a preferred set notifies no one" (PP-k3km.3). An agent following the skill would tell the user that the owner and watchers will be notified. PERMANENT is still the right class, because turning a personal set into a community set can't be undone, so give that as the reason and drop the notification claim.

3. Low: the denied-before-any-write path for preferred changes is untested (src/test/integration/mcp-tools.test.ts:4637)
The member-denied case sends only preferredTournament. If preferredChanges is removed from hasCurateChange (line 167), every test still passes because the service denies anyway. Send a content edit too (a plain member editing their own personal set with name + preferredHouse: true) and assert nothing was written.

—Claude (Claude-SessionOrchestrator)

[timothyfroehlich]

Addressed all three findings from the review of 28f4ab46f in commit 85e0caea9:

1. Tag removals ordered after preferred changes (High):
   Reordered the mutation loops in runUpdateSettingsSet so that tag additions run first, followed by setPreferredSettingsSet, and then tag removals run last. This enables calls that clear preferred and untag in one request ({ house: false, preferredHouse: false }) to successfully clear preferred before tag removal is validated, preventing any half-written content changes. Added an integration test specifically covering this combination.

2. Dropped notification claims in docs (Medium):
   Updated plugins/pinpoint/skills/pinpoint-mcp/SKILL.md and plugins/pinpoint/skills/pinpoint-mcp/references/tools.md to remove the claim that preferred changes notify watchers (aligned with spec 4.6 known divergence / PP-k3km.3). Retained PERMANENT class with the rationale that turning a personal set into a community set cannot be undone and is visible on the machine timeline.

3. Pre-write denial integration test for regular members (Low):
   Updated the regular-member test in src/test/integration/mcp-tools.test.ts to have a member edit their personal set with both a content change (name: "Member Attempted Rename") and preferredTournament: true. Verified that the call is denied up front, writing neither the name change nor any timeline events.

Also merged latest origin/main, with all static gates (pnpm run check) and integration tests passing.

[timothyfroehlich]

Claude Code review (medium)

Reviewed head 85e0cae with /code-review medium. No findings.

—Claude