diff --git a/package.json b/package.json index 785795ee37..09f6f12b89 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,8 @@ "clean": "pnpm -r run clean && rm -rf node_modules", "check:start-profile-isolation": "node --test scripts/start-dev-profile-isolation.test.mjs", "check:brand-dictionary": "node --test scripts/brand-dictionary-helper.test.mjs", - "check:brand-guard": "node --test scripts/intake-from-opensource.test.mjs" + "check:brand-guard": "node --test scripts/intake-from-opensource.test.mjs", + "check:biome-version": "node -e \"const pkg=require('./package.json');const v=pkg.devDependencies?.['@biomejs/biome']??'?';console.log('biome lockfile version:',v)\"" }, "devDependencies": { "@biomejs/biome": "^2.4.1", diff --git a/packages/api/src/config/capabilities/capability-write-guards.ts b/packages/api/src/config/capabilities/capability-write-guards.ts index 48d060b08b..26b8d7da64 100644 --- a/packages/api/src/config/capabilities/capability-write-guards.ts +++ b/packages/api/src/config/capabilities/capability-write-guards.ts @@ -21,13 +21,26 @@ export function resolveCapabilityWriteSessionUserId(request: FastifyRequest): st return typeof sessionUserId === 'string' && sessionUserId.trim() ? sessionUserId.trim() : null; } +/** + * Owner gate for capability/plugin write operations (#794 unified pattern). + * + * **For write routes** (install, delete, toggle, config save): + * Use `{ allowMissingOwner: true }` — falls through in local single-user mode + * (no DEFAULT_OWNER_USER_ID configured). Security is provided by session auth + + * origin check + loopback guard; the owner gate only adds value when explicitly + * configured for multi-user/shared deployments. + * + * **For data-visibility filters** (canReadSensitiveMcpConfig): + * Use `{ requireConfiguredOwner: true }` — fail when unconfigured, because + * "not configured" means "hide sensitive data" not "block the request". + * + * ⚠️ Do NOT use requireConfiguredOwner for write routes — it blocks local single-user + * mode unnecessarily. This mistake has recurred multiple times (#995, #794 history). + */ export function requireCapabilityWriteOwner( userId: string, options: CapabilityWriteOwnerOptions = {}, ): CapabilityWriteRouteError | null { - // Write callers (MCP install/delete, plugin enable/disable) pass allowMissingOwner: true - // → fall through in single-user mode. The data-filter caller (canReadSensitiveMcpConfig) - // passes requireConfiguredOwner: true → fail when unconfigured (sensitive data gate). const shouldFallThrough = !!options.allowMissingOwner && !options.requireConfiguredOwner; return resolveOwnerGate(userId, { requireConfiguredOwner: !shouldFallThrough, diff --git a/packages/api/src/domains/cats/services/agents/providers/acp/AcpProcessPool.ts b/packages/api/src/domains/cats/services/agents/providers/acp/AcpProcessPool.ts index da7ebcf067..392486426a 100644 --- a/packages/api/src/domains/cats/services/agents/providers/acp/AcpProcessPool.ts +++ b/packages/api/src/domains/cats/services/agents/providers/acp/AcpProcessPool.ts @@ -69,6 +69,8 @@ interface AcpPoolVariantConfig { interface PoolEntry { client: AcpPoolClient; leaseCount: number; + /** Bumped on stale-lease force-release so old lease closures become no-ops (#992). */ + leaseGeneration: number; lastUsedAt: number; state: 'initializing' | 'ready' | 'closing'; idleTimer: ReturnType | null; @@ -144,7 +146,22 @@ export class AcpProcessPool { if (this.supportsMultiplexing || owner.leaseCount === 0) { return this.leaseReadyEntry(owner, poolKey); } - throw new Error(`ACP session ${sessionId} is already active on its owning process`); + // #992: Stale lease recovery — the previous lease holder is a zombie (e.g. Windows + // console disconnect where the async generator finally block never ran). Since the + // caller is re-acquiring the SAME sessionId, the previous consumer is necessarily + // gone. Force-release the orphaned lease so the process can be reused. + log.warn( + { poolKey, sessionId, staleLeaseCount: owner.leaseCount }, + 'ACP stale lease detected — force-releasing zombie lease for session re-acquire', + ); + this._metrics.activeLeaseCount -= owner.leaseCount; + owner.leaseCount = 0; + // Transition to idle so leaseReadyEntry's idleProcessCount-- is balanced. + this._metrics.idleProcessCount++; + // Bump generation so any late-arriving release() from the old lease becomes a + // no-op (the old closure captured the previous generation value). + owner.leaseGeneration++; + return this.leaseReadyEntry(owner, poolKey); } if (owner) this.sessionOwners.delete(sessionKey); } @@ -268,12 +285,19 @@ export class AcpProcessPool { private createLease(entry: PoolEntry, poolKey: PoolKey): AcpLease { let released = false; + // Capture the generation at lease creation time. If a stale-lease force-release + // bumps the generation before this closure runs, the release becomes a no-op — + // preventing the late-arriving old finally from corrupting the new lease (#992). + const creationGeneration = entry.leaseGeneration; return { client: entry.client, poolKey, release: () => { if (released) return; released = true; + // Stale lease guard: generation mismatch means this lease was force-released + // and a new lease has been issued on the same entry. The old release is a no-op. + if (entry.leaseGeneration !== creationGeneration) return; entry.leaseCount--; this._metrics.activeLeaseCount--; if (entry.leaseCount <= 0) { @@ -290,6 +314,7 @@ export class AcpProcessPool { const entry: PoolEntry = { client, leaseCount: 0, // caller manages lease count after spawn + leaseGeneration: 0, lastUsedAt: Date.now(), state: 'initializing', idleTimer: null, diff --git a/packages/api/src/infrastructure/email/ReviewFeedbackTaskSpec.ts b/packages/api/src/infrastructure/email/ReviewFeedbackTaskSpec.ts index 97ca20babc..be3922d1cf 100644 --- a/packages/api/src/infrastructure/email/ReviewFeedbackTaskSpec.ts +++ b/packages/api/src/infrastructure/email/ReviewFeedbackTaskSpec.ts @@ -17,7 +17,6 @@ import { type Thread, } from '../../domains/cats/services/stores/ports/ThreadStore.js'; import type { ICommunityEventLog } from '../../domains/community/CommunityEventLog.js'; -import { decideDelivery } from '../../domains/community/community-delivery-policy.js'; import type { DistillationCheckpoint } from '../distillation/DistillationCheckpoint.js'; import type { ExecuteContext, TaskSpec_P1 } from '../scheduler/types.js'; import type { ConnectorInvokeTrigger, ConnectorTriggerPolicy } from './ConnectorInvokeTrigger.js'; @@ -505,33 +504,18 @@ export function createReviewFeedbackTaskSpec(opts: ReviewFeedbackTaskSpecOptions const reviewFilter = opts.isEchoReview; // R5-P2: use safeDeliveryXxx (items up to first failure) so items after a break are // not notified this round — they will be retried next poll without double-notification. + // #1002: decideDelivery removed — it silenced OWNER/MEMBER reviews, + // but PR tracking is opt-in (cat explicitly registered), so ALL reviewer + // feedback should be delivered. isEchoComment + isNoiseComment are sufficient. const newComments = safeDeliveryComments.filter((c) => { if (commentFilter?.(c)) return false; if (noiseFilter?.(c)) return false; - // F168 Phase B: apply delivery policy — OWNER/MEMBER activity is silent-log - const decision = decideDelivery({ - state: 'in_progress', // stateless function — state field not used - eventKind: 'pr.review_submitted', - authorAssociation: c.authorAssociation as - | import('@cat-cafe/shared').GitHubAuthorAssociation - | undefined, - }); - if (decision === 'silent-log') return false; return true; }); - const newDecisions = ( - reviewFilter ? safeDeliveryReviews.filter((r) => !reviewFilter(r)) : safeDeliveryReviews - ).filter((r) => { - // F168 Phase B: apply delivery policy — OWNER/MEMBER review decisions are silent-log - const decision = decideDelivery({ - state: 'in_progress', // stateless function — state field not used - eventKind: 'pr.review_submitted', - authorAssociation: r.authorAssociation as - | import('@cat-cafe/shared').GitHubAuthorAssociation - | undefined, - }); - return decision !== 'silent-log'; - }); + // #1002: same — isEchoReview is the only filter needed for review decisions. + const newDecisions = reviewFilter + ? safeDeliveryReviews.filter((r) => !reviewFilter(r)) + : safeDeliveryReviews; // R4-P1-B: when eventLog is configured, cap cursor advancement at the last // successfully projected item (maxSafeXxxCursor). Items beyond a projection diff --git a/packages/api/src/routes/connector-plugins.ts b/packages/api/src/routes/connector-plugins.ts index e1dea582bf..c791efe7da 100644 --- a/packages/api/src/routes/connector-plugins.ts +++ b/packages/api/src/routes/connector-plugins.ts @@ -21,6 +21,7 @@ import type { FastifyPluginAsync, FastifyReply, FastifyRequest } from 'fastify'; import { type CapabilityWriteRouteError, requireCapabilityWriteOwner, + requireLocalCapabilityWriteRequest, } from '../config/capabilities/capability-write-guards.js'; import { configEventBus, createChangeSetId } from '../config/config-event-bus.js'; import { isOriginAllowed, PRIVATE_NETWORK_ORIGIN, resolveFrontendCorsOrigins } from '../config/frontend-origin.js'; @@ -70,6 +71,11 @@ function resolvePluginIconMime(filePath: string): string | null { } function requirePluginWriteAccess(request: FastifyRequest): CapabilityWriteRouteError | null { + // Layer 1: Loopback guard — reject non-localhost and proxy-forwarded requests (#794 pattern). + const localError = requireLocalCapabilityWriteRequest(request); + if (localError) return localError; + + // Layer 2: Session authentication. const userId = resolveSessionUserId(request); if (!userId) { return { status: 401, error: 'Plugin writes require session authentication' }; @@ -80,13 +86,16 @@ function requirePluginWriteAccess(request: FastifyRequest): CapabilityWriteRoute return { status: 403, error: 'Connector plugin writes require same-origin Hub access' }; } - return requireCapabilityWriteOwner(userId, { - requireConfiguredOwner: true, - missingOwnerError: 'Connector plugin writes require DEFAULT_OWNER_USER_ID to be configured', - }); + // Layer 3: Owner gate — fall through in single-user mode (#794 pattern, #995 fix). + return requireCapabilityWriteOwner(userId, { allowMissingOwner: true }); } function requirePluginListAccess(request: FastifyRequest): CapabilityWriteRouteError | null { + // Layer 1: Loopback guard — plugin metadata is reconnaissance-sensitive (#794 pattern). + const localError = requireLocalCapabilityWriteRequest(request); + if (localError) return localError; + + // Layer 2: Session authentication. const userId = resolveSessionUserId(request); if (!userId) { return { status: 401, error: 'Plugin listing requires session authentication' }; @@ -97,10 +106,8 @@ function requirePluginListAccess(request: FastifyRequest): CapabilityWriteRouteE return { status: 403, error: 'Connector plugin listing requires same-origin Hub access' }; } - return requireCapabilityWriteOwner(userId, { - requireConfiguredOwner: true, - missingOwnerError: 'Connector plugin listing requires DEFAULT_OWNER_USER_ID to be configured', - }); + // Layer 3: Owner gate — fall through in single-user mode (#794 pattern, #995 fix). + return requireCapabilityWriteOwner(userId, { allowMissingOwner: true }); } function toPublicPluginMeta(plugins: ReturnType) { diff --git a/packages/api/src/skills/skill-sync-config.ts b/packages/api/src/skills/skill-sync-config.ts index 82a7f875d9..000ae8b105 100644 --- a/packages/api/src/skills/skill-sync-config.ts +++ b/packages/api/src/skills/skill-sync-config.ts @@ -136,12 +136,20 @@ export interface ConfigSyncCtx { pruneMountPaths?: boolean; /** When true, inherited-only global mount paths are skipped to preserve cascade. */ preserveGlobalCascade?: boolean; + /** Skill names that already exist in the project config (used with preserveGlobalCascade + * to distinguish existing skills from newly discovered ones — fixes #991). */ + existingProjectSkills?: ReadonlySet; /** Mount point IDs that were just enabled (absent in previous rules, present now). * When set, active skills (mountPaths.length > 0) get these IDs supplemented. */ newlyEnabledMountPointIds?: string[]; } export async function updateConfigAfterSync(projectRoot: string, ctx: ConfigSyncCtx): Promise { + // #991: New skills discovered during drift-resolve that inherit global mount paths. + // They need a config entry (so drift-check stops reporting configNew) but must NOT + // have project-local mountPaths written (that would freeze the global cascade — #962). + const cascadeNewSkills: string[] = []; + if (ctx.enabledNames.length > 0) { const grouped = new Map(); const noPolicySkills: string[] = []; @@ -157,7 +165,14 @@ export async function updateConfigAfterSync(projectRoot: string, ctx: ConfigSync // Only active in drift-resolve context (preserveGlobalCascade=true) where global // policy changes should propagate without freezing. Explicit sync operations // (sync/sync-skill) write mount paths to establish local baseline. - if (ctx.preserveGlobalCascade && !hasLocalPolicy && !ctx.mountPathsBySkill.has(name)) continue; + if (ctx.preserveGlobalCascade && !hasLocalPolicy && !ctx.mountPathsBySkill.has(name)) { + // #991: New skills still need a config entry so drift-check stops reporting + // configNew. Collect them for registration WITHOUT mountPaths below. + if (ctx.existingProjectSkills && !ctx.existingProjectSkills.has(name)) { + cascadeNewSkills.push(name); + } + continue; + } const shouldPrune = ctx.pruneMountPaths || !hasLocalPolicy; const mountPointIds = shouldPrune ? declared.filter((id) => activeSet.has(id)) : [...declared]; // F228: When a mount point is newly enabled, supplement active skills @@ -181,6 +196,28 @@ export async function updateConfigAfterSync(projectRoot: string, ctx: ConfigSync await updateSkillMountPaths(projectRoot, noPolicySkills, ctx.activeTargetIds); } } + // #991: Register new inherited-only skills in config WITHOUT mountPaths. + // This creates a managed capability entry (drift-check stops reporting configNew) + // while preserving global mount path cascade (no project-local mountPaths written, + // so resolveEffectiveSkillMountPaths falls through to global — #962 intent kept). + if (cascadeNewSkills.length > 0) { + const config = await readCapabilitiesConfig(projectRoot); + if (config) { + const existingIds = new Set( + config.capabilities + .filter((c) => c.type === 'skill' && c.source === 'cat-cafe' && !c.pluginId) + .map((c) => c.id), + ); + let changed = false; + for (const name of cascadeNewSkills) { + if (!existingIds.has(name)) { + config.capabilities.push({ id: name, type: 'skill', source: 'cat-cafe', enabled: true, globalEnabled: true }); + changed = true; + } + } + if (changed) await writeCapabilitiesConfig(projectRoot, config); + } + } if (ctx.removedNames.length > 0) { const disabledDirs = STANDARD_MOUNT_POINT_IDS.filter((id) => !ctx.mountRules.mountPoints[id].enabled).map((id) => join(projectRoot, ctx.mountRules.mountPoints[id].path), diff --git a/packages/api/src/skills/skill-sync-engine.ts b/packages/api/src/skills/skill-sync-engine.ts index 3ebab116f1..8454750225 100644 --- a/packages/api/src/skills/skill-sync-engine.ts +++ b/packages/api/src/skills/skill-sync-engine.ts @@ -408,6 +408,7 @@ async function syncProjectUnlocked( mountRules, pruneMountPaths: opts.pruneMountPaths, preserveGlobalCascade: opts.preserveGlobalCascade, + existingProjectSkills: new Set(previousNames), newlyEnabledMountPointIds: opts.pruneMountPaths ? newlyEnabledMountPointIds : undefined, }); diff --git a/packages/api/test/acp/acp-process-pool.test.js b/packages/api/test/acp/acp-process-pool.test.js index e17d1fe360..174001c118 100644 --- a/packages/api/test/acp/acp-process-pool.test.js +++ b/packages/api/test/acp/acp-process-pool.test.js @@ -216,6 +216,77 @@ describe('AcpProcessPool', () => { resumeLease.release(); }); + test('stale lease on session-owned entry is force-released on re-acquire (#992)', async () => { + const { AcpProcessPool } = await import( + '../../dist/domains/cats/services/agents/providers/acp/AcpProcessPool.js' + ); + pool = new AcpProcessPool(defaultPoolConfig, nonMultiplexedVariantConfig, createMockClient); + + // Simulate: first acquire + rememberSession, but lease never released (zombie) + const lease1 = await pool.acquire(key1); + const ownerClient = lease1.client; + pool.rememberSession(key1, 'stale-sess', lease1); + // Do NOT release lease1 — simulates Windows console disconnect where finally never runs + + assert.strictEqual(pool.getMetrics().activeLeaseCount, 1); + + // Second acquire with same sessionId should recover, not throw + const lease2 = await pool.acquire(key1, { sessionId: 'stale-sess' }); + assert.strictEqual(lease2.client, ownerClient, 'should reuse the same process'); + assert.ok(lease2.client.isAlive); + + // The stale lease was force-released, and a new lease was granted + // activeLeaseCount should be 1 (the new lease), not 2 + assert.strictEqual(pool.getMetrics().activeLeaseCount, 1); + + lease2.release(); + assert.strictEqual(pool.getMetrics().activeLeaseCount, 0); + }); + + test('late release of stale lease does not corrupt new lease (#992 P1)', async () => { + const { AcpProcessPool } = await import( + '../../dist/domains/cats/services/agents/providers/acp/AcpProcessPool.js' + ); + pool = new AcpProcessPool( + { ...defaultPoolConfig, idleTtlMs: 20, healthCheckIntervalMs: 999_999 }, + nonMultiplexedVariantConfig, + createMockClient, + ); + + // Step 1: acquire lease1, remember session, don't release (zombie) + const lease1 = await pool.acquire(key1); + const ownerClient = lease1.client; + pool.rememberSession(key1, 'late-sess', lease1); + + // Step 2: re-acquire same session → force-release recovery + const lease2 = await pool.acquire(key1, { sessionId: 'late-sess' }); + assert.strictEqual(lease2.client, ownerClient); + assert.strictEqual(pool.getMetrics().activeLeaseCount, 1); + + // Step 3: old lease1.release() arrives late (async generator finally fires) + lease1.release(); + + // Invariants that must hold after late release: + // - new lease2 is still active (not corrupted) + assert.ok(lease2.client.isAlive, 'new lease client must still be alive'); + // - activeLeaseCount must not go negative + assert.ok(pool.getMetrics().activeLeaseCount >= 0, 'activeLeaseCount must not go negative'); + // - activeLeaseCount should still be 1 (lease2 is active, lease1's release was stale) + assert.strictEqual(pool.getMetrics().activeLeaseCount, 1, 'late stale release must be no-op'); + // - idleProcessCount must not go negative + assert.ok(pool.getMetrics().idleProcessCount >= 0, 'idleProcessCount must not go negative'); + + // Step 4: wait past idle TTL — process must NOT be evicted while lease2 is active + await new Promise((r) => setTimeout(r, 50)); + assert.ok(lease2.client.isAlive, 'lease2 client must survive idle TTL'); + assert.strictEqual(pool.getMetrics().liveProcessCount, 1); + + // Step 5: normal release of lease2 should work correctly + lease2.release(); + assert.strictEqual(pool.getMetrics().activeLeaseCount, 0); + assert.strictEqual(pool.getMetrics().idleProcessCount, 1); + }); + test('double release is safe (no-op)', async () => { const { AcpProcessPool } = await import( '../../dist/domains/cats/services/agents/providers/acp/AcpProcessPool.js' diff --git a/packages/api/test/connector-plugins-route.test.js b/packages/api/test/connector-plugins-route.test.js index a7991cde47..7ad7bdfc4c 100644 --- a/packages/api/test/connector-plugins-route.test.js +++ b/packages/api/test/connector-plugins-route.test.js @@ -212,12 +212,13 @@ describe('POST /api/connectors/plugins/install auth boundary', () => { } }); - it('requires DEFAULT_OWNER_USER_ID before plugin install', async () => { + it('allows single-user mode plugin install without DEFAULT_OWNER_USER_ID (#995)', async () => { clearOwnerUserId(); setFrontendUrl('http://localhost:3003'); const app = await buildPluginRouteApp(); try { + // Auth passes through (allowMissingOwner: true), reaches file validation const res = await app.inject({ method: 'POST', url: '/api/connectors/plugins/install', @@ -225,11 +226,13 @@ describe('POST /api/connectors/plugins/install auth boundary', () => { 'x-test-session-user': 'single-user', origin: 'http://localhost:3003', host: 'localhost:3003', + 'content-type': 'multipart/form-data; boundary=test-boundary', }, + payload: '--test-boundary--\r\n', }); - assert.equal(res.statusCode, 403); - assert.match(res.body, /DEFAULT_OWNER_USER_ID|configured owner/i); + assert.equal(res.statusCode, 400); + assert.match(res.body, /No file uploaded/); } finally { await app.close(); } @@ -263,6 +266,7 @@ describe('POST /api/connectors/plugins/install auth boundary', () => { const app = await buildPluginRouteApp(); try { + // Non-loopback origin is caught by the loopback guard (layer 1) before origin check. const res = await app.inject({ method: 'POST', url: '/api/connectors/plugins/install', @@ -274,18 +278,20 @@ describe('POST /api/connectors/plugins/install auth boundary', () => { }); assert.equal(res.statusCode, 403); - assert.match(res.body, /same-origin|origin/i); + assert.match(res.body, /localhost/i); } finally { await app.close(); } }); - it('allows configured owner from trusted frontend origin through to upload validation', async () => { + it('blocks remote clients from plugin install even with configured owner', async () => { setOwnerUserId('owner-user'); setFrontendUrl('https://hub.example.test'); const app = await buildPluginRouteApp(); try { + // Remote IP is caught by the loopback guard (layer 1) — connector plugin writes + // are direct-local Hub surface only (#794 three-layer pattern). const res = await app.inject({ method: 'POST', url: '/api/connectors/plugins/install', @@ -299,8 +305,69 @@ describe('POST /api/connectors/plugins/install auth boundary', () => { remoteAddress: '203.0.113.10', }); - assert.equal(res.statusCode, 400); - assert.match(res.body, /No file uploaded/); + assert.equal(res.statusCode, 403); + assert.match(res.body, /localhost/i); + } finally { + await app.close(); + } + }); + + it('rejects plugin install when proxy forwarding headers are present', async () => { + clearOwnerUserId(); + setFrontendUrl('http://localhost:3003'); + const app = await buildPluginRouteApp(); + + try { + // Loopback peer IP + loopback origin, but X-Forwarded-For header present → + // loopback guard rejects (proxy-forwarded requests are untrusted for writes). + const res = await app.inject({ + method: 'POST', + url: '/api/connectors/plugins/install', + headers: { + 'x-test-session-user': 'single-user', + origin: 'http://localhost:3003', + host: 'localhost:3003', + 'x-forwarded-for': '10.0.0.5', + }, + }); + + assert.equal(res.statusCode, 403); + assert.match(res.body, /localhost/i); + } finally { + await app.close(); + } + }); + + it('rejects plugin list when proxy forwarding headers are present', async () => { + clearOwnerUserId(); + setFrontendUrl('http://localhost:3003'); + const root = useTempConfigRoot(); + const pluginDir = join(root, '.cat-cafe', 'plugins', 'listed-plugin'); + mkdirSync(pluginDir, { recursive: true }); + writeFileSync( + join(pluginDir, 'connector.yaml'), + 'id: listed-plugin\nname: Listed Plugin\nconfig: []\nsteps:\n - text: Step\n', + ); + writeFileSync(join(pluginDir, 'index.js'), 'export default {};\n'); + + const app = await buildPluginRouteApp(); + + try { + // Same as above — proxy forwarding headers trigger loopback guard rejection. + const res = await app.inject({ + method: 'GET', + url: '/api/connectors/plugins', + headers: { + 'x-test-session-user': 'single-user', + origin: 'http://localhost:3003', + host: 'localhost:3003', + 'x-forwarded-for': '10.0.0.5', + }, + }); + + assert.equal(res.statusCode, 403); + assert.match(res.body, /localhost/i); + assert.doesNotMatch(res.body, /listed-plugin/); } finally { await app.close(); } @@ -331,12 +398,13 @@ describe('DELETE /api/connectors/plugins/:id auth boundary', () => { } }); - it('allows configured owner from trusted frontend origin through to uninstall lookup', async () => { + it('blocks remote clients from plugin uninstall even with configured owner', async () => { setOwnerUserId('owner-user'); setFrontendUrl('https://hub.example.test'); const app = await buildPluginRouteApp(); try { + // Remote IP blocked by loopback guard (layer 1). const res = await app.inject({ method: 'DELETE', url: '/api/connectors/plugins/missing-plugin', @@ -348,8 +416,8 @@ describe('DELETE /api/connectors/plugins/:id auth boundary', () => { remoteAddress: '203.0.113.10', }); - assert.equal(res.statusCode, 404); - assert.match(res.body, /not installed/); + assert.equal(res.statusCode, 403); + assert.match(res.body, /localhost/i); } finally { await app.close(); } @@ -372,7 +440,12 @@ describe('GET /api/connectors/plugins', () => { await app.ready(); try { - const res = await app.inject({ method: 'GET', url: '/api/connectors/plugins' }); + // Provide loopback origin so layer 1 (loopback guard) passes, then layer 2 (session) rejects. + const res = await app.inject({ + method: 'GET', + url: '/api/connectors/plugins', + headers: { origin: 'http://localhost:3003', host: 'localhost:3003' }, + }); assert.equal(res.statusCode, 401); assert.match(res.body, /session/i); @@ -382,7 +455,7 @@ describe('GET /api/connectors/plugins', () => { } }); - it('requires DEFAULT_OWNER_USER_ID before listing installed plugins', async () => { + it('allows single-user mode plugin listing without DEFAULT_OWNER_USER_ID (#995)', async () => { clearOwnerUserId(); const root = useTempConfigRoot(); const pluginDir = join(root, '.cat-cafe', 'plugins', 'listed-plugin'); @@ -396,15 +469,21 @@ describe('GET /api/connectors/plugins', () => { const app = await buildPluginRouteApp(); try { + // All three layers pass: loopback (localhost origin) + session + owner (allowMissingOwner) const res = await app.inject({ method: 'GET', url: '/api/connectors/plugins', - headers: { 'x-test-session-user': 'single-user' }, + headers: { + 'x-test-session-user': 'single-user', + origin: 'http://localhost:3003', + host: 'localhost:3003', + }, }); - assert.equal(res.statusCode, 403); - assert.match(res.body, /DEFAULT_OWNER_USER_ID|configured owner/i); - assert.doesNotMatch(res.body, /listed-plugin/); + assert.equal(res.statusCode, 200); + const body = JSON.parse(res.body); + assert.equal(body.plugins.length, 1); + assert.equal(body.plugins[0].id, 'listed-plugin'); } finally { await app.close(); } @@ -424,10 +503,15 @@ describe('GET /api/connectors/plugins', () => { const app = await buildPluginRouteApp(); try { + // Loopback passes (localhost origin), session passes, owner gate rejects. const res = await app.inject({ method: 'GET', url: '/api/connectors/plugins', - headers: { 'x-test-session-user': 'other-user' }, + headers: { + 'x-test-session-user': 'other-user', + origin: 'http://localhost:3003', + host: 'localhost:3003', + }, }); assert.equal(res.statusCode, 403); @@ -452,6 +536,7 @@ describe('GET /api/connectors/plugins', () => { const app = await buildPluginRouteApp(); try { + // Non-loopback origin is caught by loopback guard (layer 1) before origin check. const res = await app.inject({ method: 'GET', url: '/api/connectors/plugins', @@ -463,7 +548,7 @@ describe('GET /api/connectors/plugins', () => { }); assert.equal(res.statusCode, 403); - assert.match(res.body, /same-origin|origin/i); + assert.match(res.body, /localhost/i); assert.doesNotMatch(res.body, /listed-plugin/); } finally { await app.close(); @@ -472,7 +557,7 @@ describe('GET /api/connectors/plugins', () => { it('does not expose installed plugin filesystem paths', async () => { setOwnerUserId('owner-user'); - setFrontendUrl('https://hub.example.test'); + setFrontendUrl('http://localhost:3003'); const root = useTempConfigRoot(); const pluginDir = join(root, '.cat-cafe', 'plugins', 'listed-plugin'); mkdirSync(pluginDir, { recursive: true }); @@ -491,15 +576,15 @@ describe('GET /api/connectors/plugins', () => { await app.ready(); try { + // Use localhost (loopback) origin so all three auth layers pass. const res = await app.inject({ method: 'GET', url: '/api/connectors/plugins', headers: { 'x-test-session-user': 'owner-user', - origin: 'https://hub.example.test', - host: 'api.example.test', + origin: 'http://localhost:3003', + host: 'localhost:3003', }, - remoteAddress: '203.0.113.10', }); assert.equal(res.statusCode, 200); diff --git a/packages/api/test/f168-phase-b-review-feedback-delivery.test.js b/packages/api/test/f168-phase-b-review-feedback-delivery.test.js index e5ccb480a2..f2c0ed45f6 100644 --- a/packages/api/test/f168-phase-b-review-feedback-delivery.test.js +++ b/packages/api/test/f168-phase-b-review-feedback-delivery.test.js @@ -1,15 +1,13 @@ /** - * F168 Phase B — R2-P1-A: ReviewFeedbackTaskSpec delivery policy tests + * F168 Phase B — ReviewFeedbackTaskSpec delivery policy tests * - * Verifies that decideDelivery() is called in ReviewFeedbackTaskSpec for both - * PR review decisions and inline/conversation comments — OWNER/MEMBER activity - * must be silent-logged (not routed to owner thread). + * #1002 fix: decideDelivery() removed from ReviewFeedbackTaskSpec. PR review + * tracking is opt-in (cat explicitly registered), so ALL reviewer feedback + * should be delivered regardless of authorAssociation. The existing + * isEchoComment + isNoiseComment + isEchoReview filters are sufficient. * - * R2 finding: ReviewFeedbackTaskSpec routes ALL reviews/comments to workItems - * unconditionally; OWNER/MEMBER maintainer activity still wakes the owner, - * violating F168 Phase B's awaiting_external suppression spec. - * - * RED tests written first; GREEN after implementation. + * Previously (R2-P1-A), OWNER/MEMBER reviews were silent-logged — this was + * wrong because it caused maintainer reviews to be silently dropped (#1002). */ import assert from 'node:assert/strict'; @@ -98,15 +96,15 @@ async function runGate(spec) { // Tests: delivery policy in ReviewFeedbackTaskSpec // --------------------------------------------------------------------------- -describe('ReviewFeedbackTaskSpec: delivery policy — R2-P1-A', () => { - it('OWNER review decision is filtered (silent-logged) and NOT routed', async () => { +describe('ReviewFeedbackTaskSpec: delivery policy — #1002 fix', () => { + it('OWNER review decision IS delivered (#1002: decideDelivery removed)', async () => { assert.ok(createReviewFeedbackTaskSpec, 'module must be importable'); const taskStore = makeTaskStore(); taskStore.addTask(makePrTask()); const router = makeReviewFeedbackRouter(); const decisions = [ - // External reviewer — should be delivered + // External reviewer — delivered { id: 101, author: 'external-reviewer', @@ -115,7 +113,7 @@ describe('ReviewFeedbackTaskSpec: delivery policy — R2-P1-A', () => { submittedAt: '2026-01-01T00:00:00Z', authorAssociation: 'CONTRIBUTOR', }, - // Repo owner reviewing own PR — should be silent-logged + // Repo owner reviewing — now also delivered (#1002) { id: 102, author: 'repo-owner', @@ -137,20 +135,20 @@ describe('ReviewFeedbackTaskSpec: delivery policy — R2-P1-A', () => { const gate = await runGate(spec); - assert.strictEqual(gate.run, true, 'gate should run — CONTRIBUTOR review is deliverable'); + assert.strictEqual(gate.run, true, 'gate should run — both reviews are deliverable'); const decisionIds = gate.workItems.flatMap((wi) => wi.signal.newDecisions.map((d) => d.id)); assert.ok(decisionIds.includes(101), 'CONTRIBUTOR review (id=101) must be in work items'); - assert.ok(!decisionIds.includes(102), 'OWNER review (id=102) must be silent-logged, not delivered'); + assert.ok(decisionIds.includes(102), 'OWNER review (id=102) must be delivered (#1002 fix)'); }); - it('MEMBER inline comment is filtered (silent-logged) and NOT routed', async () => { + it('MEMBER inline comment IS delivered (#1002: decideDelivery removed)', async () => { assert.ok(createReviewFeedbackTaskSpec); const taskStore = makeTaskStore(); taskStore.addTask(makePrTask()); const router = makeReviewFeedbackRouter(); const comments = [ - // External user inline comment — should be delivered + // External user inline comment — delivered { id: 201, author: 'external-user', @@ -159,7 +157,7 @@ describe('ReviewFeedbackTaskSpec: delivery policy — R2-P1-A', () => { commentType: 'inline', authorAssociation: 'NONE', }, - // Org member inline comment — should be silent-logged + // Org member inline comment — now also delivered (#1002) { id: 202, author: 'org-member', @@ -183,7 +181,7 @@ describe('ReviewFeedbackTaskSpec: delivery policy — R2-P1-A', () => { const commentIds = gate.workItems.flatMap((wi) => wi.signal.newComments.map((c) => c.id)); assert.ok(commentIds.includes(201), 'external comment (id=201) must be delivered'); - assert.ok(!commentIds.includes(202), 'MEMBER comment (id=202) must be silent-logged'); + assert.ok(commentIds.includes(202), 'MEMBER comment (id=202) must be delivered (#1002 fix)'); }); it('undefined authorAssociation defaults to wake-owner (conservative)', async () => { @@ -211,7 +209,7 @@ describe('ReviewFeedbackTaskSpec: delivery policy — R2-P1-A', () => { assert.ok(decisionIds.includes(301), 'review with no authorAssociation must default to wake-owner'); }); - it('mixed scenario: only external activity reaches router', async () => { + it('mixed scenario: ALL reviewer activity reaches router (#1002)', async () => { assert.ok(createReviewFeedbackTaskSpec); const taskStore = makeTaskStore(); taskStore.addTask(makePrTask()); @@ -269,8 +267,8 @@ describe('ReviewFeedbackTaskSpec: delivery policy — R2-P1-A', () => { const decisionIds = gate.workItems.flatMap((wi) => wi.signal.newDecisions.map((d) => d.id)); assert.ok(commentIds.includes(401), 'external comment must be delivered'); - assert.ok(!commentIds.includes(402), 'OWNER comment must be silent-logged'); + assert.ok(commentIds.includes(402), 'OWNER comment must be delivered (#1002 fix)'); assert.ok(decisionIds.includes(501), 'COLLABORATOR review must be delivered'); - assert.ok(!decisionIds.includes(502), 'MEMBER review must be silent-logged'); + assert.ok(decisionIds.includes(502), 'MEMBER review must be delivered (#1002 fix)'); }); }); diff --git a/packages/api/test/f168-phase-b-review-feedback-event-log.test.js b/packages/api/test/f168-phase-b-review-feedback-event-log.test.js index 0420952912..93572537f3 100644 --- a/packages/api/test/f168-phase-b-review-feedback-event-log.test.js +++ b/packages/api/test/f168-phase-b-review-feedback-event-log.test.js @@ -273,7 +273,7 @@ describe('ReviewFeedbackTaskSpec: event log append — polling fallback (R3-P1)' ); }); - it('OWNER review is still appended to event log (silent-log = still captured)', async () => { + it('OWNER review is appended to event log AND delivered (#1002)', async () => { assert.ok(createReviewFeedbackTaskSpec); const taskStore = makeTaskStore(makePrTask()); const eventLog = makeEventLog({ appended: true }); @@ -303,13 +303,13 @@ describe('ReviewFeedbackTaskSpec: event log append — polling fallback (R3-P1)' const gate = await runGate(spec); - // OWNER is silent-logged: NOT in workItems (delivery filter) + // #1002 fix: OWNER review IS now delivered (decideDelivery removed) const deliveredIds = (gate.run ? (gate.workItems ?? []) : []).flatMap((wi) => wi.signal.newDecisions.map((d) => d.id), ); - assert.ok(!deliveredIds.includes(501), 'OWNER review must not be delivered'); + assert.ok(deliveredIds.includes(501), 'OWNER review must be delivered (#1002 fix)'); - // But IS appended to event log (state machine must see it) + // AND appended to event log (state machine must see it) const ownerAppend = eventLog.appendCalls.find((e) => e.payload?.reviewId === 501); assert.ok(ownerAppend, 'OWNER review must still be appended to event log'); assert.strictEqual(ownerAppend.payload.authorAssociation, 'OWNER'); diff --git a/packages/api/test/scheduler/review-feedback-thread-rotation.test.js b/packages/api/test/scheduler/review-feedback-thread-rotation.test.js index d1faa53a94..a644195f09 100644 --- a/packages/api/test/scheduler/review-feedback-thread-rotation.test.js +++ b/packages/api/test/scheduler/review-feedback-thread-rotation.test.js @@ -245,7 +245,7 @@ describe('#949 / F140: review feedback returns to the registered thread', () => assert.equal(threadStore._createCalls.length, 0, 'legacy repair must not create another thread'); }); - it('delivers routing audit when legacy repair feedback is otherwise filtered', async () => { + it('delivers routing audit alongside OWNER feedback (#1002: no longer filtered)', async () => { const { createReviewFeedbackTaskSpec } = await import('../../dist/infrastructure/email/ReviewFeedbackTaskSpec.js'); const task = mockTask( { repoFullName: 'owner/repo', prNumber: 48, catId: 'opus', threadId: 'thread_rotated_1', userId: 'u-1' }, @@ -303,10 +303,12 @@ describe('#949 / F140: review feedback returns to the registered thread', () => previousThreadId: 'thread_rotated_1', repairedThreadId: 'th-original', }); - assert.deepEqual(calls[0].signal.newComments, [], 'filtered feedback should not be reintroduced'); + // #1002: OWNER comments are now delivered (decideDelivery removed) + assert.equal(calls[0].signal.newComments.length, 1, 'OWNER comment must be delivered (#1002)'); + assert.equal(calls[0].signal.newComments[0].id, 34); assert.equal(calls[0].tracking.threadId, 'th-original'); const cursorPatch = store._patchCalls.find((call) => call.patch.review?.lastCommentCursor !== undefined); - assert.ok(cursorPatch, 'audit delivery should still commit the filtered feedback cursor'); + assert.ok(cursorPatch, 'cursor must advance past delivered comment'); assert.equal(cursorPatch.patch.review.lastCommentCursor, 34); }); diff --git a/packages/web/src/components/settings/SkillsContent.tsx b/packages/web/src/components/settings/SkillsContent.tsx index 66912280f5..8090e91726 100644 --- a/packages/web/src/components/settings/SkillsContent.tsx +++ b/packages/web/src/components/settings/SkillsContent.tsx @@ -215,11 +215,6 @@ export function SkillsContent() { }} /> - )} @@ -237,28 +232,35 @@ export function SkillsContent() { /> )} - {scope === SCOPE_ALL && data && ( - - )} - - {data && filteredSkills.some((s) => s.controls) && ( -
-

- {batchEnabled ? '批量禁用当前筛选的 Skill' : '批量启用当前筛选的 Skill'} -

- handleBatchToggle(!batchEnabled)} - title={batchEnabled ? '批量禁用当前筛选的 Skill' : '批量启用当前筛选的 Skill'} - /> + {data && ( +
+
+ {scope === SCOPE_ALL && ( + + )} + {scope === SCOPE_PROJECT && ( + + )} +
+ {filteredSkills.some((s) => s.controls) && ( + handleBatchToggle(!batchEnabled)} + title={batchEnabled ? '批量禁用当前筛选的 Skill' : '批量启用当前筛选的 Skill'} + /> + )}
)} diff --git a/packages/web/src/hooks/__tests__/useAgentMessages-background-system-info-web-search.test.ts b/packages/web/src/hooks/__tests__/useAgentMessages-background-system-info-web-search.test.ts index b75270b206..b2a547b37e 100644 --- a/packages/web/src/hooks/__tests__/useAgentMessages-background-system-info-web-search.test.ts +++ b/packages/web/src/hooks/__tests__/useAgentMessages-background-system-info-web-search.test.ts @@ -566,6 +566,126 @@ describe('consumeBackgroundSystemInfo liveness_warning', () => { }); }); +// #966: provider_capability background handler — mirror of foreground handler. +// Without this handler, kimi invocations in background threads surface raw-JSON bubbles. +describe('consumeBackgroundSystemInfo provider_capability (#966)', () => { + it('consumes provider_capability silently (no raw JSON bubble)', () => { + const options = createMockOptions(); + const msg = { + type: 'system_info', + catId: 'kimi', + threadId: 'thread-bg', + content: JSON.stringify({ + type: 'provider_capability', + capability: 'thinking', + status: 'unavailable', + provider: 'kimi', + reason: 'kimi-cli 本次流式输出未提供可解析的 think/reasoning 内容', + }), + timestamp: Date.now(), + }; + + const result = consumeBackgroundSystemInfo(msg, undefined, options); + + expect(result.consumed).toBe(true); + expect(options.store.addMessageToThread).not.toHaveBeenCalled(); + expect(options.store.setThreadCatInvocation).toHaveBeenCalledWith('thread-bg', 'kimi', { + providerCapabilities: { + thinking: expect.objectContaining({ + status: 'unavailable', + provider: 'kimi', + reason: expect.any(String), + }), + }, + }); + }); + + it('merges multiple capabilities without clobbering (read-merge-write)', () => { + const options = createMockOptions({ + getThreadState: vi.fn(() => ({ + messages: [], + catStatuses: {}, + catInvocations: { + kimi: { + providerCapabilities: { + thinking: { status: 'unavailable', provider: 'kimi', reason: 'n/a', receivedAt: 1000 }, + }, + }, + }, + })), + }); + const msg = { + type: 'system_info', + catId: 'kimi', + threadId: 'thread-bg', + content: JSON.stringify({ + type: 'provider_capability', + capability: 'image_input', + status: 'limited', + provider: 'kimi', + reason: 'max 4 images', + }), + timestamp: Date.now(), + }; + + const result = consumeBackgroundSystemInfo(msg, undefined, options); + + expect(result.consumed).toBe(true); + const call = vi.mocked(options.store.setThreadCatInvocation).mock.calls[0]; + expect(call?.[2]?.providerCapabilities).toMatchObject({ + thinking: { status: 'unavailable' }, + image_input: { status: 'limited', provider: 'kimi' }, + }); + }); + + it('coerces unknown status to unavailable', () => { + const options = createMockOptions(); + const msg = { + type: 'system_info', + catId: 'kimi', + threadId: 'thread-bg', + content: JSON.stringify({ + type: 'provider_capability', + capability: 'thinking', + status: 'bogus', + provider: 'kimi', + reason: '', + }), + timestamp: Date.now(), + }; + + const result = consumeBackgroundSystemInfo(msg, undefined, options); + + expect(result.consumed).toBe(true); + const call = vi.mocked(options.store.setThreadCatInvocation).mock.calls[0]; + expect(call?.[2]?.providerCapabilities?.thinking?.status).toBe('unavailable'); + }); + + it('uses msg.catId fallback when parsed.catId is empty string', () => { + const options = createMockOptions(); + const msg = { + type: 'system_info', + catId: 'kimi', + threadId: 'thread-bg', + content: JSON.stringify({ + type: 'provider_capability', + catId: '', + capability: 'thinking', + status: 'unavailable', + provider: 'kimi', + reason: '', + }), + timestamp: Date.now(), + }; + + const result = consumeBackgroundSystemInfo(msg, undefined, options); + + expect(result.consumed).toBe(true); + // Should use msg.catId='kimi' because parsed.catId='' is falsy with || + expect(options.store.setThreadCatInvocation).toHaveBeenCalledWith('thread-bg', 'kimi', expect.any(Object)); + }); +}); + describe('consumeBackgroundSystemInfo warning + telemetry suppression', () => { it('converts warning JSON to readable text (not raw JSON bubble)', () => { const options = createMockOptions(); diff --git a/scripts/intake-from-opensource.sh b/scripts/intake-from-opensource.sh index 076e09db6c..fff6f1d87f 100755 --- a/scripts/intake-from-opensource.sh +++ b/scripts/intake-from-opensource.sh @@ -192,27 +192,40 @@ is_high_risk() { # Format: file|check_type|pattern|description # check_type: must_not_contain, must_contain, file_exists # Both --validate-inbound and pre-commit hook consume this list. +# +# TODO(brand-parameterization): These rules were mirrored from cat-cafe verbatim. +# In the clowder-ai repo, "Clowder AI" IS the correct brand (per brand-dictionary.yaml). +# Brand-sensitive must_not_contain/must_contain entries below are disabled until +# per-repo parameterization is implemented. See PR #993 review discussion. BRAND_EXPECTATIONS=( # layout.tsx - "packages/web/src/app/layout.tsx|must_not_contain|Clowder AI|title should be Clowder AI" - "packages/web/src/app/layout.tsx|must_not_contain|Your AI team collaboration space|description should be Chinese" + # DISABLED: Clowder AI is clowder-ai's brand, not contamination + # "packages/web/src/app/layout.tsx|must_not_contain|Clowder AI|title should be Clowder AI" + # "packages/web/src/app/layout.tsx|must_not_contain|Your AI team collaboration space|description should be Chinese" "packages/web/src/app/layout.tsx|must_contain|favicon.svg|favicon declaration required" "packages/web/src/app/layout.tsx|must_contain|icon-192x192.png|PWA icon declaration required" # SplitPaneView.tsx - "packages/web/src/components/SplitPaneView.tsx|must_not_contain|Clowder AI|brand should be Clowder AI" - # manifest.json - "packages/web/public/manifest.json|must_not_contain|Clowder|name should be Clowder AI" + # DISABLED: same reason + # "packages/web/src/components/SplitPaneView.tsx|must_not_contain|Clowder AI|brand should be Clowder AI" + # manifest.json — keep a benign file_exists entry so Phase 2 dictionary scan + # exempts this file (Phase 2 skips files already in BRAND_EXPECTATIONS). + # Brand content rules disabled; parameterization needed. + "packages/web/public/manifest.json|file_exists|manifest.json|PWA manifest must exist" + # "packages/web/public/manifest.json|must_not_contain|Clowder|name should be Clowder AI" # ChatContainerHeader.tsx — surface text AND semantic fields - "packages/web/src/components/ChatContainerHeader.tsx|must_not_contain|Clowder AI|brand should be Clowder AI" - "packages/web/src/components/ChatContainerHeader.tsx|must_contain|Cat Caf|must have Clowder AI brand" + # DISABLED: same reason + # "packages/web/src/components/ChatContainerHeader.tsx|must_not_contain|Clowder AI|brand should be Clowder AI" + # "packages/web/src/components/ChatContainerHeader.tsx|must_contain|Cat Caf|must have Clowder AI brand" "packages/web/src/components/ChatContainerHeader.tsx|must_contain|'cat-cafe'|INTERNAL_BASENAMES must include cat-cafe" "packages/web/src/components/ChatContainerHeader.tsx|must_contain|'cat-cafe-runtime'|INTERNAL_BASENAMES must include cat-cafe-runtime" # api-client.ts — comment AND real brand identity (F156: header → session cookie) - "packages/web/src/utils/api-client.ts|must_not_contain|client for Clowder AI|comment should reference Clowder AI" + # DISABLED: same reason + # "packages/web/src/utils/api-client.ts|must_not_contain|client for Clowder AI|comment should reference Clowder AI" "packages/web/src/utils/api-client.ts|must_contain|HttpOnly session cookie|identity uses session cookie (F156 D-1)" # connector command deep links — home runtime fallback must stay on 3001; public sync transforms it to 3003. - "packages/api/src/infrastructure/connectors/connector-gateway-bootstrap.ts|must_not_contain|http://localhost:3003|connector command fallback should use Clowder AI frontend port" - "packages/api/src/infrastructure/connectors/connector-gateway-bootstrap.ts|must_contain|http://localhost:3003|connector command fallback should use Clowder AI frontend port" + # DISABLED: contradictory pair (must_not + must_contain same value); needs per-repo parameterization + # "packages/api/src/infrastructure/connectors/connector-gateway-bootstrap.ts|must_not_contain|http://localhost:3003|connector command fallback should use Clowder AI frontend port" + # "packages/api/src/infrastructure/connectors/connector-gateway-bootstrap.ts|must_contain|http://localhost:3003|connector command fallback should use Clowder AI frontend port" # adapter media URLs — must not hardcode opensource ports (3003/3004); use API_SERVER_PORT env fallback. # Outbound sync transforms 3002→3004; intake must catch un-reversed port references. "packages/api/src/infrastructure/connectors/im-connectors/weixin/WeixinAdapter.ts|must_not_contain|localhost:3004|Weixin media fallback should use runtime API_SERVER_PORT not hardcoded opensource port" @@ -436,7 +449,9 @@ run_brand_validation() { while IFS= read -r bsf; do [ -z "$bsf" ] && continue # Skip files already checked by BRAND_EXPECTATIONS (avoid double-counting) - if echo "${BRAND_EXPECTATIONS[*]}" | grep -q "^${bsf}|"; then continue; fi + # Fix: use printf + [@] so each entry gets its own line; the old echo + [*] + # joined everything on one line, making ^anchor match only the first entry. + if printf '%s\n' "${BRAND_EXPECTATIONS[@]}" | grep -q "^${bsf}|"; then continue; fi # Skip brand-validation toolchain files — they reference brand terms as # detection constants, not as content that needs sanitization. case "$bsf" in diff --git a/scripts/intake-from-opensource.test.mjs b/scripts/intake-from-opensource.test.mjs index 7831ccdaaf..5bef51734b 100644 --- a/scripts/intake-from-opensource.test.mjs +++ b/scripts/intake-from-opensource.test.mjs @@ -591,15 +591,17 @@ if (flag === '--classify-path') { assert.match(err.stdout, /fail-closed|broken.*brand-sensitive|anchor.*pet/i); }); - it('catches public frontend port contamination in connector-gateway-bootstrap.ts', () => { + it('allows port 3003 in connector-gateway-bootstrap.ts (clowder-ai public port)', () => { + // PR #993: BRAND_EXPECTATIONS entry for connector-gateway-bootstrap.ts was + // disabled — 3003 is the correct public frontend port for the clowder-ai + // repo (not contamination). Brand guard must NOT flag it. const f = makeBrandFixture({ 'packages/api/src/infrastructure/connectors/connector-gateway-bootstrap.ts': "frontendBaseUrl: deps.frontendBaseUrl ?? 'http://localhost:3003',", }); fixtures.push(f.sandboxRoot); - const err = captureValidateFailure(f.repoRoot); - assert.match(err.stdout, /brand violation/i); - assert.match(err.stdout, /connector-gateway-bootstrap/); + const output = runValidate(f.repoRoot); + assert.match(output, /No brand violations detected/); }); it('scopes standalone validation to local changed files in a git worktree', () => {