import { describe, it, expect, beforeEach } from 'vitest';
import express from 'express';
import request from 'supertest';
import { mkdtempSync, writeFileSync, mkdirSync, existsSync, readFileSync } from 'fs';
import { join } from 'path';
import { tmpdir } from 'os';
import { mountPiecesApi } from './pieces-api.js';

function makeGeneralPieceYaml(): string {
  return [
    'name: general',
    'description: 汎用タスク',
    'max_movements: 25',
    'initial_movement: understand',
    'movements:',
    '  - name: understand',
    '    edit: false',
    '    persona: analyst',
    '    instruction: |',
    '      タスクを確認する。',
    '    allowed_tools: [Read, Glob]',
    '    default_next: execute',
    '    rules:',
    '      - condition: 方針が立った',
    '        next: execute',
    '  - name: execute',
    '    edit: true',
    '    persona: worker',
    '    instruction: |',
    '      作業を実行する。',
    '    allowed_tools: [Read, Write]',
    '    default_next: COMPLETE',
    '    rules:',
    '      - condition: 完了',
    '        next: COMPLETE',
  ].join('\n');
}

function makeMinimalPieceYaml(name: string, description = 'x'): string {
  return [
    `name: ${name}`,
    `description: ${description}`,
    'max_movements: 1',
    'initial_movement: only',
    'movements:',
    '  - name: only',
    '    edit: false',
    '    persona: p',
    '    instruction: i',
    '    allowed_tools: [Read]',
    '    default_next: COMPLETE',
    '    rules: []',
  ].join('\n');
}

describe('Pieces API (no auth — legacy behavior)', () => {
  let app: express.Application;
  let piecesDir: string;

  beforeEach(() => {
    const tempDir = mkdtempSync(join(tmpdir(), 'pieces-api-'));
    piecesDir = join(tempDir, 'pieces');
    mkdirSync(piecesDir);
    writeFileSync(join(piecesDir, 'general.yaml'), makeGeneralPieceYaml());
    app = express();
    app.use(express.json());
    mountPiecesApi(app, { piecesDir });
  });

  it('GET /api/pieces returns piece list', async () => {
    const res = await request(app).get('/api/pieces');
    expect(res.status).toBe(200);
    expect(res.body.pieces).toHaveLength(1);
    expect(res.body.pieces[0].name).toBe('general');
    expect(res.body.pieces[0].source).toBe('builtin');
    expect(res.body.pieces[0].custom).toBe(false);
  });

  it('GET /api/pieces/:name returns full piece', async () => {
    const res = await request(app).get('/api/pieces/general');
    expect(res.status).toBe(200);
    expect(res.body.piece.name).toBe('general');
    expect(res.body.piece.movements).toHaveLength(2);
    expect(res.body.source).toBe('builtin');
  });

  it('GET /api/pieces/:name returns 404 for unknown', async () => {
    const res = await request(app).get('/api/pieces/nonexistent');
    expect(res.status).toBe(404);
  });

  it('PUT /api/pieces/:name updates piece', async () => {
    const res = await request(app)
      .put('/api/pieces/general')
      .send({
        name: 'general',
        description: '更新済み',
        max_movements: 30,
        initial_movement: 'understand',
        movements: [
          { name: 'understand', edit: false, persona: 'analyst', instruction: 'テスト', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] },
        ],
      });
    expect(res.status).toBe(200);
    expect(res.body.ok).toBe(true);
  });

  it('POST /api/pieces creates new piece', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'custom',
        description: 'カスタム',
        max_movements: 10,
        initial_movement: 'work',
        movements: [
          { name: 'work', edit: true, persona: 'worker', instruction: '作業する', allowed_tools: ['Read', 'Write'], default_next: 'COMPLETE', rules: [] },
        ],
      });
    expect(res.status).toBe(201);
    expect(res.body.ok).toBe(true);
  });

  it('POST /api/pieces rejects duplicate name', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({ name: 'general', description: 'x', max_movements: 1, initial_movement: 'a', movements: [{ name: 'a', edit: false, persona: 'x', instruction: 'x', allowed_tools: [], rules: [] }] });
    expect(res.status).toBe(409);
  });

  it('POST /api/pieces rejects rules[].next: COMPLETE (Phase 6b)', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'phase6b-reject',
        description: 'should be rejected',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          { name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'],
            rules: [{ condition: 'ok', next: 'COMPLETE' }] },
        ],
      });
    expect(res.status).toBe(400);
    expect(String(res.body.error ?? res.body)).toMatch(/rules\[\]\.next cannot be "COMPLETE"/);
  });

  it('POST /api/pieces accepts default_next: COMPLETE (engine-internal sentinel)', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'phase6b-default-ok',
        description: 'default_next is fine',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          { name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'],
            default_next: 'COMPLETE', rules: [] },
        ],
      });
    expect(res.status).toBe(201);
  });

  // Phase 4: per-movement SSH connection allowlist validation.
  it('POST /api/pieces rejects SshExec without allowed_ssh_connections', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'ssh-missing-allowlist',
        description: 'x',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          { name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['SshExec'],
            default_next: 'COMPLETE', rules: [] },
        ],
      });
    expect(res.status).toBe(400);
    expect(String(res.body.error ?? res.body)).toMatch(/allowed_ssh_connections is required/);
  });

  it('POST /api/pieces accepts SshExec with UUID allowlist', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'ssh-uuid-ok',
        description: 'x',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          {
            name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['SshExec'],
            allowed_ssh_connections: ['6f9619ff-8b86-d011-b42d-00c04fc964ff'],
            default_next: 'COMPLETE', rules: [],
          },
        ],
      });
    expect(res.status).toBe(201);
  });

  it('POST /api/pieces accepts SshExec with ["*"] wildcard', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'ssh-wildcard-ok',
        description: 'x',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          {
            name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['SshExec'],
            allowed_ssh_connections: ['*'],
            default_next: 'COMPLETE', rules: [],
          },
        ],
      });
    expect(res.status).toBe(201);
  });

  it('POST /api/pieces accepts SshExec with empty allowlist (explicit deny)', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'ssh-empty-ok',
        description: 'x',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          {
            name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['SshExec'],
            allowed_ssh_connections: [],
            default_next: 'COMPLETE', rules: [],
          },
        ],
      });
    expect(res.status).toBe(201);
  });

  it('POST /api/pieces rejects allowed_ssh_connections with bad format', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'ssh-bad-format',
        description: 'x',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          {
            name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['SshExec'],
            allowed_ssh_connections: ['BAD-NOT-LOWERCASE'],
            default_next: 'COMPLETE', rules: [],
          },
        ],
      });
    expect(res.status).toBe(400);
    expect(String(res.body.error ?? res.body)).toMatch(/must be '\*' or a lowercase hex/);
  });

  it('POST /api/pieces rejects non-array allowed_ssh_connections', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'ssh-non-array',
        description: 'x',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          {
            name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['SshExec'],
            allowed_ssh_connections: 'not-an-array',
            default_next: 'COMPLETE', rules: [],
          },
        ],
      });
    expect(res.status).toBe(400);
    expect(String(res.body.error ?? res.body)).toMatch(/must be an array/);
  });

  it('POST /api/pieces accepts allowed_ssh_connections without SSH tools (no-op)', async () => {
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'ssh-noop',
        description: 'x',
        max_movements: 1,
        initial_movement: 'only',
        movements: [
          {
            name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'],
            allowed_ssh_connections: ['6f9619ff-8b86-d011-b42d-00c04fc964ff'],
            default_next: 'COMPLETE', rules: [],
          },
        ],
      });
    expect(res.status).toBe(201);
  });

  it('DELETE /api/pieces/:name — legacy no-auth pieces land in piecesDir (builtin source) and are non-deletable', async () => {
    // In legacy no-auth mode with no customPiecesDir, POST writes to piecesDir.
    // Those files are resolved as source='builtin' and are non-deletable.
    await request(app).post('/api/pieces').send({
      name: 'deleteme', description: 'x', max_movements: 1, initial_movement: 'a',
      movements: [{ name: 'a', edit: false, persona: 'x', instruction: 'x', allowed_tools: [], default_next: 'COMPLETE', rules: [] }],
    });
    const res = await request(app).delete('/api/pieces/deleteme');
    expect(res.status).toBe(403);
    expect(res.body.error).toMatch(/cannot delete a built-in/i);
  });

  it('DELETE /api/pieces/general is forbidden', async () => {
    const res = await request(app).delete('/api/pieces/general');
    expect(res.status).toBe(403);
  });
});

// ---------------------------------------------------------------------------
// Auth-aware tests (per-user custom pieces + non-admin write authz)
// ---------------------------------------------------------------------------

type UserShape = { id: string; role: 'admin' | 'user' };

function makeAuthApp(piecesDir: string, userPiecesRootDir: string, user: UserShape | null): express.Application {
  const app = express();
  app.use(express.json());
  app.use((req, _res, next) => {
    if (user) (req as any).user = user;
    next();
  });
  mountPiecesApi(app, { piecesDir, userPiecesRootDir });
  return app;
}

describe('Pieces API (auth-aware: per-user custom + write authz)', () => {
  let piecesDir: string;
  let userPiecesRootDir: string;

  beforeEach(() => {
    const tempDir = mkdtempSync(join(tmpdir(), 'pieces-api-auth-'));
    piecesDir = join(tempDir, 'pieces');
    userPiecesRootDir = join(tempDir, 'users');
    mkdirSync(piecesDir);
    mkdirSync(userPiecesRootDir);
    writeFileSync(join(piecesDir, 'general.yaml'), makeGeneralPieceYaml());
    writeFileSync(join(piecesDir, 'chat.yaml'), makeMinimalPieceYaml('chat', 'built-in chat'));
  });

  it('GET /api/pieces returns built-ins for any authenticated non-admin', async () => {
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .get('/api/pieces');
    expect(res.status).toBe(200);
    expect(res.body.pieces.map((p: any) => p.name).sort()).toEqual(['chat', 'general']);
    for (const p of res.body.pieces) {
      expect(p.source).toBe('builtin');
      expect(p.custom).toBe(false);
    }
  });

  it('GET /api/pieces merges caller\'s user-custom pieces (own only)', async () => {
    // Alice has my-tool, Bob has bob-tool
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'my-tool.yaml'), makeMinimalPieceYaml('my-tool', "alice's piece"));
    mkdirSync(join(userPiecesRootDir, 'bob', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'bob', 'pieces', 'bob-tool.yaml'), makeMinimalPieceYaml('bob-tool', "bob's piece"));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .get('/api/pieces');
    expect(res.status).toBe(200);
    const byName = Object.fromEntries(res.body.pieces.map((p: any) => [p.name, p]));
    expect(Object.keys(byName).sort()).toEqual(['chat', 'general', 'my-tool']);
    expect(byName['my-tool'].source).toBe('user-custom');
    expect(byName['my-tool'].ownerId).toBe('alice');
    expect(byName['my-tool'].custom).toBe(true);
    // Bob's piece must not appear for Alice
    expect(byName['bob-tool']).toBeUndefined();
  });

  it("GET /api/pieces — user-custom and built-in both appear when same name (no hiding)", async () => {
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'general.yaml'), makeMinimalPieceYaml('general', 'alice override'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .get('/api/pieces');
    expect(res.status).toBe(200);
    const generals = res.body.pieces.filter((p: any) => p.name === 'general');
    // Both builtin and user-custom should appear
    expect(generals).toHaveLength(2);
    const customGeneral = generals.find((p: any) => p.source === 'user-custom');
    const builtinGeneral = generals.find((p: any) => p.source === 'builtin');
    expect(customGeneral).toBeDefined();
    expect(customGeneral.description).toBe('alice override');
    expect(builtinGeneral).toBeDefined();
  });

  it('POST /api/pieces creates a user-custom piece for non-admin caller', async () => {
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .post('/api/pieces')
      .send({
        name: 'alice-custom',
        description: 'alice piece',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(201);
    expect(existsSync(join(userPiecesRootDir, 'alice', 'pieces', 'alice-custom.yaml'))).toBe(true);
    // Built-in dir untouched
    expect(existsSync(join(piecesDir, 'alice-custom.yaml'))).toBe(false);
  });

  it('POST /api/pieces by admin writes to user-custom dir (not piecesDir)', async () => {
    // POST always targets user-custom dir regardless of admin role.
    // Admins edit built-ins via PUT on existing ones.
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'admin1', role: 'admin' }))
      .post('/api/pieces')
      .send({
        name: 'admin-piece',
        description: 'admin piece',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(201);
    // Piece lands in admin's user-custom dir, NOT in piecesDir.
    expect(existsSync(join(userPiecesRootDir, 'admin1', 'pieces', 'admin-piece.yaml'))).toBe(true);
    expect(existsSync(join(piecesDir, 'admin-piece.yaml'))).toBe(false);
  });

  it('PUT /api/pieces/:name on built-in by non-admin returns 403', async () => {
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .put('/api/pieces/general')
      .send({
        name: 'general',
        description: 'should not write',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(403);
  });

  it('PUT /api/pieces/:name on own user-custom by owner returns 200', async () => {
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'my-piece.yaml'), makeMinimalPieceYaml('my-piece', 'v1'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .put('/api/pieces/my-piece')
      .send({
        name: 'my-piece',
        description: 'v2',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(200);
  });

  it("PUT /api/pieces/:name — non-admin cannot edit another user's piece", async () => {
    // Bob has a piece. Alice tries to edit it via the same name.
    mkdirSync(join(userPiecesRootDir, 'bob', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'bob', 'pieces', 'bob-only.yaml'), makeMinimalPieceYaml('bob-only', 'bob'));

    // Alice doesn't have bob-only — the request 404s (she can't see it), proving isolation.
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .put('/api/pieces/bob-only')
      .send({
        name: 'bob-only',
        description: 'hijacked',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(404);
  });

  it('DELETE /api/pieces/:name on built-in by non-admin returns 403', async () => {
    // Use a deletable built-in (not general/chat which are protected separately)
    writeFileSync(join(piecesDir, 'extra.yaml'), makeMinimalPieceYaml('extra', 'extra'));
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .delete('/api/pieces/extra');
    expect(res.status).toBe(403);
    expect(existsSync(join(piecesDir, 'extra.yaml'))).toBe(true);
  });

  it('DELETE /api/pieces/:name on own user-custom by owner returns 200', async () => {
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'goner.yaml'), makeMinimalPieceYaml('goner', 'gone'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .delete('/api/pieces/goner');
    expect(res.status).toBe(200);
    expect(existsSync(join(userPiecesRootDir, 'alice', 'pieces', 'goner.yaml'))).toBe(false);
  });

  it('admin can edit built-in pieces (PUT 200) but NOT delete them (DELETE 403)', async () => {
    writeFileSync(join(piecesDir, 'admin-target.yaml'), makeMinimalPieceYaml('admin-target', 'before'));
    const adminApp = makeAuthApp(piecesDir, userPiecesRootDir, { id: 'admin1', role: 'admin' });

    // Admin CAN edit (PUT) a built-in
    const putRes = await request(adminApp).put('/api/pieces/admin-target').send({
      name: 'admin-target',
      description: 'after',
      max_movements: 1,
      initial_movement: 'only',
      movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
    });
    expect(putRes.status).toBe(200);

    // Admin CANNOT delete a built-in — 403 for everyone
    const delRes = await request(adminApp).delete('/api/pieces/admin-target');
    expect(delRes.status).toBe(403);
    expect(delRes.body.ok).toBe(false);
    expect(delRes.body.error).toMatch(/cannot delete a built-in/i);
    // File must still exist
    expect(existsSync(join(piecesDir, 'admin-target.yaml'))).toBe(true);
  });

  // --- Task 2A: built-in must not be hidden by same-named custom ---
  it("GET /api/pieces — built-in is NOT hidden when user has a same-named custom", async () => {
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'), makeMinimalPieceYaml('chat', 'alice chat override'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .get('/api/pieces');
    expect(res.status).toBe(200);
    // Both the user-custom AND the builtin should appear
    const chatPieces = res.body.pieces.filter((p: any) => p.name === 'chat');
    expect(chatPieces).toHaveLength(2);
    const sources = chatPieces.map((p: any) => p.source).sort();
    expect(sources).toEqual(['builtin', 'user-custom']);
    // Custom has the custom description, builtin has the original
    const customChat = chatPieces.find((p: any) => p.source === 'user-custom');
    const builtinChat = chatPieces.find((p: any) => p.source === 'builtin');
    expect(customChat.description).toBe('alice chat override');
    expect(builtinChat.description).toBe('built-in chat');
  });

  // --- Task 2B: CreatePiece rejects name collision with built-in ---
  it('POST /api/pieces rejects custom creation with a built-in name', async () => {
    // 'general' and 'chat' are in piecesDir (built-in)
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .post('/api/pieces')
      .send({
        name: 'chat',
        description: 'my chat',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(409);
    expect(res.body.error).toMatch(/built-in/i);
  });

  it('POST /api/pieces with a fresh custom name still works for non-admin', async () => {
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .post('/api/pieces')
      .send({
        name: 'fresh-custom',
        description: 'fresh',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(201);
  });

  // --- Task 2C: regression — non-admin DELETE of built-in → 403 ---
  it('non-admin DELETE of built-in returns 403', async () => {
    writeFileSync(join(piecesDir, 'deletable-builtin.yaml'), makeMinimalPieceYaml('deletable-builtin', 'del'));
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .delete('/api/pieces/deletable-builtin');
    expect(res.status).toBe(403);
    expect(existsSync(join(piecesDir, 'deletable-builtin.yaml'))).toBe(true);
  });

  // --- Fix 2: GET /api/pieces/:name?source=builtin fetches the specific source ---
  it('GET /api/pieces/:name?source=builtin returns builtin even when user-custom exists with same name', async () => {
    // Alice has a user-custom 'chat' that overrides by default priority
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'), makeMinimalPieceYaml('chat', 'alice custom chat'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .get('/api/pieces/chat?source=builtin');
    expect(res.status).toBe(200);
    expect(res.body.source).toBe('builtin');
    expect(res.body.piece.description).toBe('built-in chat');
  });

  it('GET /api/pieces/:name without ?source uses priority resolution (user-custom first)', async () => {
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'), makeMinimalPieceYaml('chat', 'alice custom chat'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .get('/api/pieces/chat');
    expect(res.status).toBe(200);
    expect(res.body.source).toBe('user-custom');
    expect(res.body.piece.description).toBe('alice custom chat');
  });

  // --- Fix 3: DELETE of a user-custom named 'chat' by its owner must succeed ---
  it("DELETE /api/pieces/chat on owner's user-custom named 'chat' returns 200 (guard only blocks builtin)", async () => {
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'), makeMinimalPieceYaml('chat', 'alice custom chat'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .delete('/api/pieces/chat');
    expect(res.status).toBe(200);
    // User-custom file removed; built-in chat still present
    expect(existsSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'))).toBe(false);
    expect(existsSync(join(piecesDir, 'chat.yaml'))).toBe(true);
  });

  it('DELETE /api/pieces/chat on builtin by non-admin returns 403', async () => {
    // No user-custom for alice, so findPieceForCaller resolves to builtin
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .delete('/api/pieces/chat');
    expect(res.status).toBe(403);
    expect(existsSync(join(piecesDir, 'chat.yaml'))).toBe(true);
  });

  // --- Fix source param for PUT/DELETE (P1-b) ---

  it('PUT /api/pieces/chat?source=builtin by admin updates the builtin, not a same-named custom', async () => {
    // Alice (admin) has a user-custom chat AND there is a builtin chat.
    mkdirSync(join(userPiecesRootDir, 'admin1', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'admin1', 'pieces', 'chat.yaml'), makeMinimalPieceYaml('chat', 'admin custom chat'));
    const adminApp = makeAuthApp(piecesDir, userPiecesRootDir, { id: 'admin1', role: 'admin' });

    const res = await request(adminApp).put('/api/pieces/chat?source=builtin').send({
      name: 'chat',
      description: 'builtin updated',
      max_movements: 1,
      initial_movement: 'only',
      movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
    });
    expect(res.status).toBe(200);
    // Builtin was updated
    const builtinContent = readFileSync(join(piecesDir, 'chat.yaml'), 'utf-8');
    expect(builtinContent).toContain('builtin updated');
    // Custom is untouched
    const customContent = readFileSync(join(userPiecesRootDir, 'admin1', 'pieces', 'chat.yaml'), 'utf-8');
    expect(customContent).toContain('admin custom chat');
  });

  it('DELETE /api/pieces/chat?source=builtin by admin deletes the builtin, not same-named custom', async () => {
    // admin1 has a user-custom chat AND builtin chat
    mkdirSync(join(userPiecesRootDir, 'admin1', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'admin1', 'pieces', 'chat.yaml'), makeMinimalPieceYaml('chat', 'admin custom'));
    const adminApp = makeAuthApp(piecesDir, userPiecesRootDir, { id: 'admin1', role: 'admin' });

    const res = await request(adminApp).delete('/api/pieces/chat?source=builtin');
    // Built-in pieces are non-deletable for everyone (including admins)
    expect(res.status).toBe(403);
    // Both still intact
    expect(existsSync(join(piecesDir, 'chat.yaml'))).toBe(true);
    expect(existsSync(join(userPiecesRootDir, 'admin1', 'pieces', 'chat.yaml'))).toBe(true);
  });

  it('DELETE /api/pieces/extra?source=user-custom targets user-custom, not builtin', async () => {
    writeFileSync(join(piecesDir, 'extra.yaml'), makeMinimalPieceYaml('extra', 'builtin extra'));
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'extra.yaml'), makeMinimalPieceYaml('extra', 'alice extra'));

    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .delete('/api/pieces/extra?source=user-custom');
    expect(res.status).toBe(200);
    // User-custom removed
    expect(existsSync(join(userPiecesRootDir, 'alice', 'pieces', 'extra.yaml'))).toBe(false);
    // Builtin untouched
    expect(existsSync(join(piecesDir, 'extra.yaml'))).toBe(true);
  });

  // P2 fix: GET /api/pieces/:name (no ?source) includes source in response body
  // so the UI can derive the correct read-only state regardless of URL params.
  it('GET /api/pieces/general (no ?source) returns source: builtin in the body', async () => {
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'alice', role: 'user' }))
      .get('/api/pieces/general');
    expect(res.status).toBe(200);
    expect(res.body.piece.name).toBe('general');
    expect(res.body.source).toBe('builtin');
    expect(res.body.custom).toBe(false);
  });

  it('POST /api/pieces creates in user-custom dir for admin (not piecesDir)', async () => {
    // Regression for P2: POST always targets user-custom, admin is no exception.
    const res = await request(makeAuthApp(piecesDir, userPiecesRootDir, { id: 'admin1', role: 'admin' }))
      .post('/api/pieces')
      .send({
        name: 'admin-new-custom',
        description: 'admin custom piece',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(201);
    expect(existsSync(join(userPiecesRootDir, 'admin1', 'pieces', 'admin-new-custom.yaml'))).toBe(true);
    expect(existsSync(join(piecesDir, 'admin-new-custom.yaml'))).toBe(false);
  });
});

// ---------------------------------------------------------------------------
// Finding 2 regression: authenticated POST with userPiecesRootDir UNSET → 503
// Never falls through to shared/builtin dirs for an authenticated user.
// ---------------------------------------------------------------------------

describe('Pieces API (Finding 2: POST auth fallback hole)', () => {
  let piecesDir: string;

  function makeAppWithAuthButNoUserPiecesRoot(user: UserShape | null): express.Application {
    const app = express();
    app.use(express.json());
    app.use((req, _res, next) => {
      if (user) (req as any).user = user;
      next();
    });
    // Intentionally: no userPiecesRootDir configured, no customPiecesDir.
    mountPiecesApi(app, { piecesDir });
    return app;
  }

  beforeEach(() => {
    const tempDir = mkdtempSync(join(tmpdir(), 'pieces-api-f2-'));
    piecesDir = join(tempDir, 'pieces');
    mkdirSync(piecesDir);
    writeFileSync(join(piecesDir, 'general.yaml'), makeGeneralPieceYaml());
  });

  it('authenticated non-admin POST with userPiecesRootDir unset returns 503 (does not write shared/builtin)', async () => {
    const app = makeAppWithAuthButNoUserPiecesRoot({ id: 'alice', role: 'user' });
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'should-not-exist',
        description: 'fallback hole test',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(503);
    expect(res.body.ok).toBe(false);
    expect(res.body.error).toMatch(/not configured/i);
    // Must NOT have written anything to the builtin dir.
    expect(existsSync(join(piecesDir, 'should-not-exist.yaml'))).toBe(false);
  });

  it('authenticated admin POST with userPiecesRootDir unset also returns 503', async () => {
    const app = makeAppWithAuthButNoUserPiecesRoot({ id: 'admin1', role: 'admin' });
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'admin-fallback-hole',
        description: 'admin hole test',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(503);
    expect(res.body.ok).toBe(false);
    // Must NOT have written anything to the builtin dir.
    expect(existsSync(join(piecesDir, 'admin-fallback-hole.yaml'))).toBe(false);
  });

  it('unauthenticated POST with no userPiecesRootDir falls back to piecesDir (legacy no-auth mode)', async () => {
    // No user: genuine no-auth legacy mode — should still work (existing behavior).
    const app = makeAppWithAuthButNoUserPiecesRoot(null);
    const res = await request(app)
      .post('/api/pieces')
      .send({
        name: 'legacy-fallback',
        description: 'legacy',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(201);
    expect(existsSync(join(piecesDir, 'legacy-fallback.yaml'))).toBe(true);
  });
});

// ---------------------------------------------------------------------------
// Fix 1: invalid ?source param → 400 (no fallback to priority resolution)
// ---------------------------------------------------------------------------

describe('Pieces API (Fix 1: invalid ?source → 400, no destructive fallback)', () => {
  let piecesDir: string;
  let userPiecesRootDir: string;

  beforeEach(() => {
    const tempDir = mkdtempSync(join(tmpdir(), 'pieces-api-fix1-'));
    piecesDir = join(tempDir, 'pieces');
    userPiecesRootDir = join(tempDir, 'users');
    mkdirSync(piecesDir);
    mkdirSync(userPiecesRootDir);
    // A builtin and a user-custom with the same name
    writeFileSync(join(piecesDir, 'chat.yaml'), makeMinimalPieceYaml('chat', 'builtin chat'));
    // Alice has a user-custom 'chat'
    mkdirSync(join(userPiecesRootDir, 'alice', 'pieces'), { recursive: true });
    writeFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'), makeMinimalPieceYaml('chat', 'alice custom chat'));
  });

  function makeApp(user: UserShape | null) {
    const app = express();
    app.use(express.json());
    app.use((req, _res, next) => {
      if (user) (req as any).user = user;
      next();
    });
    mountPiecesApi(app, { piecesDir, userPiecesRootDir });
    return app;
  }

  it('GET /api/pieces/:name?source=builtinn (typo) → 400 (not priority-fallback)', async () => {
    const res = await request(makeApp({ id: 'alice', role: 'user' }))
      .get('/api/pieces/chat?source=builtinn');
    expect(res.status).toBe(400);
    expect(res.body.ok).toBe(false);
    expect(res.body.error).toMatch(/invalid source/i);
  });

  it('DELETE /api/pieces/chat?source=builtinn (typo) → 400, does NOT delete user-custom', async () => {
    const res = await request(makeApp({ id: 'alice', role: 'user' }))
      .delete('/api/pieces/chat?source=builtinn');
    expect(res.status).toBe(400);
    expect(res.body.ok).toBe(false);
    expect(res.body.error).toMatch(/invalid source/i);
    // User-custom must be untouched
    expect(existsSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'))).toBe(true);
  });

  it('PUT /api/pieces/chat?source=user_custom (underscore typo) → 400, does NOT mutate user-custom', async () => {
    const res = await request(makeApp({ id: 'alice', role: 'user' }))
      .put('/api/pieces/chat?source=user_custom')
      .send({
        name: 'chat',
        description: 'hijacked',
        max_movements: 1,
        initial_movement: 'only',
        movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
      });
    expect(res.status).toBe(400);
    expect(res.body.ok).toBe(false);
    expect(res.body.error).toMatch(/invalid source/i);
    // User-custom description must be unchanged
    const content = readFileSync(join(userPiecesRootDir, 'alice', 'pieces', 'chat.yaml'), 'utf-8');
    expect(content).toContain('alice custom chat');
    expect(content).not.toContain('hijacked');
  });

  it('valid ?source=builtin still works (not rejected)', async () => {
    const res = await request(makeApp({ id: 'alice', role: 'user' }))
      .get('/api/pieces/chat?source=builtin');
    expect(res.status).toBe(200);
    expect(res.body.source).toBe('builtin');
  });

  it('absent ?source still does priority resolution (not rejected)', async () => {
    const res = await request(makeApp({ id: 'alice', role: 'user' }))
      .get('/api/pieces/chat');
    expect(res.status).toBe(200);
    // priority: user-custom wins
    expect(res.body.source).toBe('user-custom');
  });
});

// ---------------------------------------------------------------------------
// Fix 2: POST /api/pieces returns actual source in response body
// ---------------------------------------------------------------------------

describe('Pieces API (Fix 2: POST returns actual source)', () => {
  let piecesDir: string;
  let userPiecesRootDir: string;

  function minimalPieceBody(name: string) {
    return {
      name,
      description: 'test',
      max_movements: 1,
      initial_movement: 'only',
      movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
    };
  }

  beforeEach(() => {
    const tempDir = mkdtempSync(join(tmpdir(), 'pieces-api-fix2-'));
    piecesDir = join(tempDir, 'pieces');
    userPiecesRootDir = join(tempDir, 'users');
    mkdirSync(piecesDir);
    mkdirSync(userPiecesRootDir);
  });

  it('authenticated POST returns source: user-custom', async () => {
    const app = express();
    app.use(express.json());
    app.use((req, _res, next) => { (req as any).user = { id: 'alice', role: 'user' }; next(); });
    mountPiecesApi(app, { piecesDir, userPiecesRootDir });
    const res = await request(app).post('/api/pieces').send(minimalPieceBody('my-piece'));
    expect(res.status).toBe(201);
    expect(res.body.ok).toBe(true);
    expect(res.body.source).toBe('user-custom');
  });

  it('legacy (no-auth) POST with customPiecesDir returns source: global-custom', async () => {
    const tempDir = mkdtempSync(join(tmpdir(), 'pieces-api-fix2-gc-'));
    const gcDir = join(tempDir, 'custom');
    mkdirSync(join(tempDir, 'pieces'));
    mkdirSync(gcDir);
    const app = express();
    app.use(express.json());
    // No user set — legacy no-auth mode
    mountPiecesApi(app, { piecesDir: join(tempDir, 'pieces'), customPiecesDir: gcDir });
    const res = await request(app).post('/api/pieces').send(minimalPieceBody('gc-piece'));
    expect(res.status).toBe(201);
    expect(res.body.ok).toBe(true);
    expect(res.body.source).toBe('global-custom');
  });

  it('legacy (no-auth) POST without customPiecesDir returns source: builtin', async () => {
    const app = express();
    app.use(express.json());
    // No user, no customPiecesDir
    mountPiecesApi(app, { piecesDir });
    const res = await request(app).post('/api/pieces').send(minimalPieceBody('legacy-piece'));
    expect(res.status).toBe(201);
    expect(res.body.ok).toBe(true);
    expect(res.body.source).toBe('builtin');
  });

  it('no-auth POST with userPiecesRootDir returns source: user-custom (regression fix)', async () => {
    const app = express();
    app.use(express.json());
    // No user, but userPiecesRootDir is configured (the fixed path)
    mountPiecesApi(app, { piecesDir, userPiecesRootDir });
    const res = await request(app).post('/api/pieces').send(minimalPieceBody('noauth-custom'));
    expect(res.status).toBe(201);
    expect(res.body.ok).toBe(true);
    expect(res.body.source).toBe('user-custom');
    // Piece is in data/users/local/pieces, NOT in piecesDir
    expect(existsSync(join(userPiecesRootDir, 'local', 'pieces', 'noauth-custom.yaml'))).toBe(true);
    expect(existsSync(join(piecesDir, 'noauth-custom.yaml'))).toBe(false);
  });
});

// ---------------------------------------------------------------------------
// Regression fix: no-auth + userPiecesRootDir → 'local' user-custom namespace
// ---------------------------------------------------------------------------

describe('Pieces API (regression: no-auth uses local user-custom, not piecesDir)', () => {
  let piecesDir: string;
  let userPiecesRootDir: string;

  function minimalPieceBody(name: string) {
    return {
      name,
      description: 'test',
      max_movements: 1,
      initial_movement: 'only',
      movements: [{ name: 'only', edit: false, persona: 'p', instruction: 'i', allowed_tools: ['Read'], default_next: 'COMPLETE', rules: [] }],
    };
  }

  beforeEach(() => {
    const tempDir = mkdtempSync(join(tmpdir(), 'pieces-noauth-local-'));
    piecesDir = join(tempDir, 'pieces');
    userPiecesRootDir = join(tempDir, 'users');
    mkdirSync(piecesDir);
    mkdirSync(userPiecesRootDir);
    writeFileSync(join(piecesDir, 'general.yaml'), makeGeneralPieceYaml());
  });

  function makeNoAuthApp() {
    const app = express();
    app.use(express.json());
    // No user middleware — simulates no-auth mode
    mountPiecesApi(app, { piecesDir, userPiecesRootDir });
    return app;
  }

  it('no-auth POST creates under local user-custom dir, NOT in piecesDir', async () => {
    const app = makeNoAuthApp();
    const res = await request(app).post('/api/pieces').send(minimalPieceBody('my-noauth-piece'));
    expect(res.status).toBe(201);
    expect(res.body.source).toBe('user-custom');
    // Must be in the 'local' user-custom dir
    expect(existsSync(join(userPiecesRootDir, 'local', 'pieces', 'my-noauth-piece.yaml'))).toBe(true);
    // Must NOT be in bundled piecesDir
    expect(existsSync(join(piecesDir, 'my-noauth-piece.yaml'))).toBe(false);
  });

  it('no-auth created piece (with userPiecesRootDir) is DELETABLE — DELETE returns 200', async () => {
    const app = makeNoAuthApp();
    // Create it first
    await request(app).post('/api/pieces').send(minimalPieceBody('deletable-noauth'));
    expect(existsSync(join(userPiecesRootDir, 'local', 'pieces', 'deletable-noauth.yaml'))).toBe(true);

    // Now delete — must succeed (not 403)
    const delRes = await request(app).delete('/api/pieces/deletable-noauth');
    expect(delRes.status).toBe(200);
    expect(delRes.body.ok).toBe(true);
    expect(existsSync(join(userPiecesRootDir, 'local', 'pieces', 'deletable-noauth.yaml'))).toBe(false);
  });

  it('bundled built-in piece is still non-deletable (403) for no-auth callers', async () => {
    const app = makeNoAuthApp();
    const res = await request(app).delete('/api/pieces/general');
    expect(res.status).toBe(403);
    expect(res.body.error).toMatch(/cannot delete a built-in/i);
    expect(existsSync(join(piecesDir, 'general.yaml'))).toBe(true);
  });

  it('no-auth LIST includes the local user-custom piece (source=user-custom)', async () => {
    // Pre-create a piece in the 'local' user-custom dir
    mkdirSync(join(userPiecesRootDir, 'local', 'pieces'), { recursive: true });
    writeFileSync(
      join(userPiecesRootDir, 'local', 'pieces', 'local-custom.yaml'),
      makeMinimalPieceYaml('local-custom', 'no-auth local piece'),
    );

    const app = makeNoAuthApp();
    const res = await request(app).get('/api/pieces');
    expect(res.status).toBe(200);
    const byName = Object.fromEntries(res.body.pieces.map((p: any) => [p.name, p]));
    expect(byName['local-custom']).toBeDefined();
    expect(byName['local-custom'].source).toBe('user-custom');
    expect(byName['local-custom'].ownerId).toBe('local');
    // Built-in also visible
    expect(byName['general']).toBeDefined();
    expect(byName['general'].source).toBe('builtin');
  });

  it('no-auth GET resolves user-custom piece from local dir by priority', async () => {
    mkdirSync(join(userPiecesRootDir, 'local', 'pieces'), { recursive: true });
    writeFileSync(
      join(userPiecesRootDir, 'local', 'pieces', 'my-piece.yaml'),
      makeMinimalPieceYaml('my-piece', 'local custom'),
    );

    const app = makeNoAuthApp();
    const res = await request(app).get('/api/pieces/my-piece');
    expect(res.status).toBe(200);
    expect(res.body.source).toBe('user-custom');
    expect(res.body.ownerId).toBe('local');
  });
});