feat(KB-185): add global/project settings hierarchy with scope-aware UI
- Add GlobalSettingsStore with file-based storage in ~/.pi/kb/settings.json - Implement two-tier hierarchy: global (user prefs) and project (.kb/config.json) - Update TaskStore to merge settings with project overriding global - Add API endpoints: GET/PUT /api/settings/global and GET /api/settings/scopes - Add scope indicators (🌐 global, 📁 project) to settings modal sidebar - Implement scope-aware save: only save active scope, reject global-only fields - Add frontend API functions for global settings operations - Add tests for GlobalSettingsStore, settings routes, and SettingsModal scopes - Create changeset for the new settings hierarchy feature - Update AGENTS.md with settings hierarchy documentation
This commit is contained in:
@@ -23,6 +23,15 @@ import { isGhAuthenticated } from "@kb/core";
|
||||
|
||||
const mockIsGhAuthenticated = vi.mocked(isGhAuthenticated);
|
||||
|
||||
function createMockGlobalSettingsStore() {
|
||||
return {
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
updateSettings: vi.fn().mockResolvedValue({}),
|
||||
getSettingsPath: vi.fn().mockReturnValue("/fake/home/.pi/kb/settings.json"),
|
||||
init: vi.fn().mockResolvedValue(false),
|
||||
};
|
||||
}
|
||||
|
||||
function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||
return {
|
||||
getTask: vi.fn(),
|
||||
@@ -36,6 +45,9 @@ function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||
unarchiveTask: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
updateSettings: vi.fn(),
|
||||
updateGlobalSettings: vi.fn(),
|
||||
getSettingsByScope: vi.fn().mockResolvedValue({ global: {}, project: {} }),
|
||||
getGlobalSettingsStore: vi.fn().mockReturnValue(createMockGlobalSettingsStore()),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getAgentLogs: vi.fn().mockResolvedValue([]),
|
||||
addSteeringComment: vi.fn(),
|
||||
@@ -4487,6 +4499,50 @@ describe("PUT /settings", () => {
|
||||
expect(res.body.error).toContain("must include both provider and modelId or neither");
|
||||
});
|
||||
|
||||
it("rejects global-only fields with 400 error and helpful message", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PUT",
|
||||
"/api/settings",
|
||||
JSON.stringify({ themeMode: "dark", maxConcurrent: 4 }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("global settings");
|
||||
expect(res.body.error).toContain("themeMode");
|
||||
expect(res.body.error).toContain("/settings/global");
|
||||
});
|
||||
|
||||
it("rejects when only global fields are sent", async () => {
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PUT",
|
||||
"/api/settings",
|
||||
JSON.stringify({ defaultProvider: "anthropic", defaultModelId: "claude-sonnet-4-5" }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toContain("defaultProvider");
|
||||
});
|
||||
|
||||
it("allows project-only fields to pass through successfully", async () => {
|
||||
const updatedSettings = { ...DEFAULT_SETTINGS, maxConcurrent: 8 };
|
||||
(store.updateSettings as ReturnType<typeof vi.fn>).mockResolvedValue(updatedSettings);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PUT",
|
||||
"/api/settings",
|
||||
JSON.stringify({ maxConcurrent: 8, autoMerge: false }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateSettings).toHaveBeenCalledWith({ maxConcurrent: 8, autoMerge: false });
|
||||
});
|
||||
|
||||
it("returns 500 on store update error", async () => {
|
||||
(store.updateSettings as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Write failed"));
|
||||
|
||||
@@ -4502,3 +4558,128 @@ describe("PUT /settings", () => {
|
||||
expect(res.body.error).toContain("Write failed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("GET /settings/global", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore();
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
it("returns global settings from the global settings store", async () => {
|
||||
const mockGlobalStore = createMockGlobalSettingsStore();
|
||||
mockGlobalStore.getSettings.mockResolvedValue({ themeMode: "light", colorTheme: "ocean" });
|
||||
(store.getGlobalSettingsStore as ReturnType<typeof vi.fn>).mockReturnValue(mockGlobalStore);
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings/global");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.themeMode).toBe("light");
|
||||
expect(res.body.colorTheme).toBe("ocean");
|
||||
// Should NOT include server-only fields
|
||||
expect(res.body.githubTokenConfigured).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns 500 on global store error", async () => {
|
||||
const mockGlobalStore = createMockGlobalSettingsStore();
|
||||
mockGlobalStore.getSettings.mockRejectedValue(new Error("Read failed"));
|
||||
(store.getGlobalSettingsStore as ReturnType<typeof vi.fn>).mockReturnValue(mockGlobalStore);
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings/global");
|
||||
|
||||
expect(res.status).toBe(500);
|
||||
expect(res.body.error).toContain("Read failed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("PUT /settings/global", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore();
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
it("updates global settings via store.updateGlobalSettings", async () => {
|
||||
const updatedMerged = { themeMode: "light", maxConcurrent: 2 };
|
||||
(store.updateGlobalSettings as ReturnType<typeof vi.fn>).mockResolvedValue(updatedMerged);
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PUT",
|
||||
"/api/settings/global",
|
||||
JSON.stringify({ themeMode: "light" }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateGlobalSettings).toHaveBeenCalledWith({ themeMode: "light" });
|
||||
});
|
||||
|
||||
it("returns 500 on update error", async () => {
|
||||
(store.updateGlobalSettings as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Write failed"));
|
||||
|
||||
const res = await REQUEST(
|
||||
buildApp(),
|
||||
"PUT",
|
||||
"/api/settings/global",
|
||||
JSON.stringify({ themeMode: "light" }),
|
||||
{ "Content-Type": "application/json" },
|
||||
);
|
||||
|
||||
expect(res.status).toBe(500);
|
||||
expect(res.body.error).toContain("Write failed");
|
||||
});
|
||||
});
|
||||
|
||||
describe("GET /settings/scopes", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore();
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
it("returns settings separated by scope", async () => {
|
||||
(store.getSettingsByScope as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||
global: { themeMode: "dark", defaultProvider: "anthropic" },
|
||||
project: { maxConcurrent: 4, autoMerge: false },
|
||||
});
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings/scopes");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.global.themeMode).toBe("dark");
|
||||
expect(res.body.global.defaultProvider).toBe("anthropic");
|
||||
expect(res.body.project.maxConcurrent).toBe(4);
|
||||
expect(res.body.project.autoMerge).toBe(false);
|
||||
});
|
||||
|
||||
it("returns 500 on store error", async () => {
|
||||
(store.getSettingsByScope as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("Failed"));
|
||||
|
||||
const res = await GET(buildApp(), "/api/settings/scopes");
|
||||
|
||||
expect(res.status).toBe(500);
|
||||
expect(res.body.error).toContain("Failed");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,7 +3,7 @@ import multer from "multer";
|
||||
import { createReadStream, existsSync } from "node:fs";
|
||||
import { execSync } from "node:child_process";
|
||||
import type { TaskStore, Column, MergeResult, ScheduleType, ActivityEventType, ModelPreset } from "@kb/core";
|
||||
import { COLUMNS, VALID_TRANSITIONS, type BatchStatusEntry, type BatchStatusResponse, type BatchStatusResult, type IssueInfo, type PrInfo, isGhAuthenticated, AUTOMATION_PRESETS, AutomationStore } from "@kb/core";
|
||||
import { COLUMNS, VALID_TRANSITIONS, GLOBAL_SETTINGS_KEYS, type BatchStatusEntry, type BatchStatusResponse, type BatchStatusResult, type IssueInfo, type PrInfo, isGhAuthenticated, AUTOMATION_PRESETS, AutomationStore } from "@kb/core";
|
||||
import type { ServerOptions } from "./server.js";
|
||||
import { GitHubClient, getCurrentGitHubRepo, parseBadgeUrl } from "./github.js";
|
||||
import { githubRateLimiter } from "./github-poll.js";
|
||||
@@ -932,6 +932,16 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
const { githubTokenConfigured, ...clientSettings } = req.body;
|
||||
|
||||
// Reject global-only fields with a helpful error pointing to the correct endpoint
|
||||
const globalKeySet = new Set<string>(GLOBAL_SETTINGS_KEYS);
|
||||
const globalFieldsFound = Object.keys(clientSettings).filter((k) => globalKeySet.has(k));
|
||||
if (globalFieldsFound.length > 0) {
|
||||
res.status(400).json({
|
||||
error: `Cannot update global settings via this endpoint. Use PUT /settings/global instead. Global fields found: ${globalFieldsFound.join(", ")}`,
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (Object.prototype.hasOwnProperty.call(clientSettings, "modelPresets")) {
|
||||
clientSettings.modelPresets = validateModelPresets(clientSettings.modelPresets);
|
||||
}
|
||||
@@ -946,6 +956,51 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
}
|
||||
});
|
||||
|
||||
// ── Global Settings Routes ─────────────────────────────────────
|
||||
|
||||
/**
|
||||
* GET /api/settings/global
|
||||
* Returns the global (user-level) settings from ~/.pi/kb/settings.json.
|
||||
* Does NOT include computed/server-only fields like githubTokenConfigured.
|
||||
*/
|
||||
router.get("/settings/global", async (_req, res) => {
|
||||
try {
|
||||
const globalStore = store.getGlobalSettingsStore();
|
||||
const settings = await globalStore.getSettings();
|
||||
res.json(settings);
|
||||
} catch (err: any) {
|
||||
res.status(500).json({ error: err.message });
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* PUT /api/settings/global
|
||||
* Update global (user-level) settings in ~/.pi/kb/settings.json.
|
||||
* These settings persist across all kb projects for the current user.
|
||||
*/
|
||||
router.put("/settings/global", async (req, res) => {
|
||||
try {
|
||||
const settings = await store.updateGlobalSettings(req.body);
|
||||
res.json(settings);
|
||||
} catch (err: any) {
|
||||
res.status(500).json({ error: err.message });
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* GET /api/settings/scopes
|
||||
* Returns settings separated by scope: { global, project }.
|
||||
* Useful for the UI to show which scope each setting comes from.
|
||||
*/
|
||||
router.get("/settings/scopes", async (_req, res) => {
|
||||
try {
|
||||
const scopes = await store.getSettingsByScope();
|
||||
res.json(scopes);
|
||||
} catch (err: any) {
|
||||
res.status(500).json({ error: err.message });
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* POST /api/settings/test-ntfy
|
||||
* Send a test notification to verify ntfy configuration.
|
||||
|
||||
Reference in New Issue
Block a user