diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 89527a58..aed5f880 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -56,6 +56,12 @@ jobs: languages: ${{ matrix.language }} build-mode: ${{ matrix.build-mode }} queries: security-extended + # tests/security/fixtures/** are DELIBERATELY vulnerable planted-vuln + # samples that the security enforcement-matrix suite scans/asserts on; + # scanning them here just re-flags the intentional issues. + config: | + paths-ignore: + - tests/security/fixtures - name: Perform CodeQL Analysis uses: github/codeql-action/analyze@ce28f5bb42b7a9f2c824e633a3f6ee835bab6858 # v3.29.0 diff --git a/.github/workflows/helm-chart.yml b/.github/workflows/helm-chart.yml index 623ee85f..c12da202 100644 --- a/.github/workflows/helm-chart.yml +++ b/.github/workflows/helm-chart.yml @@ -116,6 +116,15 @@ jobs: docker build -t forge/$svc:0.1.0 -f deploy/docker/$svc.Dockerfile . done + # The chart declares postgresql/redis/minio subchart deps in Chart.yaml, so + # `helm install`/`upgrade` needs them vendored into charts/ before it runs — + # even though the kind overlay disables them (it stands up external in-cluster + # datastores). Without this, install fails "missing in charts/ directory". + - name: Add subchart repos + build chart dependencies + run: | + helm repo add bitnami https://charts.bitnami.com/bitnami + helm dependency build ${CHART} + - name: Run kind smoke tests (install + helm test + upgrade/rollback) env: FORGE_KIND_CLUSTER: forge-ci # reuse the kind-action cluster diff --git a/apps/api/forge_api/auth/service.py b/apps/api/forge_api/auth/service.py index eaaf6885..d3a80276 100644 --- a/apps/api/forge_api/auth/service.py +++ b/apps/api/forge_api/auth/service.py @@ -371,6 +371,7 @@ def resolve_model_client( workspace_id: uuid.UUID, *, secret_id: uuid.UUID | None = None, + model: str | None = None, redactor: Callable[[str], str] = redact_text, ) -> ModelClient: """Resolve a provider-agnostic BYOK :class:`ModelClient` for a workspace. @@ -382,9 +383,16 @@ def resolve_model_client( never logged. The injected ``redactor`` scrubs any provider exception before it is re-raised as ``ModelClientError``. + ``model`` overrides the env-configured model name — used by the + Adaptive Orchestration model router (``ao-model-router``) to bind a + tier-resolved model onto the workspace's provider/key without touching + any other client knob. + Raises ``ModelClientError`` when no provider is configured, and ``ModelClientUnavailable`` when the provider SDK extra is not installed. """ + import dataclasses + from forge_agent.providers import ModelClientConfig, ModelClientError, build_model_client if secret_id is not None: @@ -405,6 +413,8 @@ def resolve_model_client( "no model provider configured; set FORGE_MODEL_PROVIDER and a BYOK " "key (env or vault under MODEL_PROVIDER, + FORGE_MODEL_NAME for OpenAI)" ) + if model: + config = dataclasses.replace(config, model=model) return build_model_client(config, redactor=redactor) # -- OAuth descriptor --------------------------------------------------- # diff --git a/apps/api/forge_api/routers/saml.py b/apps/api/forge_api/routers/saml.py index 931da178..3913e21e 100644 --- a/apps/api/forge_api/routers/saml.py +++ b/apps/api/forge_api/routers/saml.py @@ -109,8 +109,21 @@ def _require_enabled(config: SsoConfiguration) -> None: def _safe_next(target: str | None) -> str: - """Only same-origin relative paths are honoured (open-redirect guard).""" - if target and target.startswith("/") and not target.startswith("//"): + """Only same-origin relative paths are honoured (open-redirect guard). + + Rejects protocol-relative (``//host``) and backslash-normalised + (``/\\host``) targets — browsers treat ``\\`` as ``/``, so ``/\\evil.com`` + would otherwise redirect off-origin — plus any embedded control characters. + """ + if ( + target + and target.startswith("/") + and not target.startswith("//") + and "\\" not in target + and "\r" not in target + and "\n" not in target + and "\t" not in target + ): return target return "/" diff --git a/apps/api/forge_api/routers/spec.py b/apps/api/forge_api/routers/spec.py index 58705a73..56533678 100644 --- a/apps/api/forge_api/routers/spec.py +++ b/apps/api/forge_api/routers/spec.py @@ -16,29 +16,43 @@ import uuid from collections.abc import Iterator from contextlib import contextmanager +from dataclasses import dataclass from functools import lru_cache from pathlib import Path -from typing import Annotated +from typing import Annotated, Any from fastapi import APIRouter, Depends, HTTPException, status +from fastapi.responses import PlainTextResponse from pydantic import BaseModel, Field from forge_api.auth.rbac import Permission from forge_api.deps import DbSession, Principal, get_current_principal from forge_api.routers._rbac import require_permission from forge_api.routers.board import BoardServiceDep +from forge_api.services import spec_version_service +from forge_api.services.spec_draft_service import SpecDraft, draft_spec +from forge_api.services.spec_import_service import SpecImport, SpecImportRequest, import_spec from forge_api.settings import get_settings from forge_contracts import ( BoardFilter, Constitution, + ModelClient, Requirement, SpecManifest, TaskDTO, ValidationReport, ) from forge_contracts.exceptions import SpecGateError -from forge_db.models import Project -from forge_spec import FileSpecEngine, SpecNotFoundError +from forge_db.models import Project, SpecVersion +from forge_orchestration_policy import Tier +from forge_spec import ( + FileSpecEngine, + ManifestDiff, + SpecNotFoundError, + diff_manifest, + diff_markdown, + spec_id_for_key, +) router = APIRouter( prefix="/spec", @@ -116,6 +130,33 @@ def _spec_errors() -> Iterator[None]: raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail=str(exc)) from exc +def _record_version( + engine: FileSpecEngine, + db: DbSession, + principal: Principal, + manifest: SpecManifest, +) -> SpecVersion: + """Snapshot ``manifest`` (+ its rendered serializations) as the next version. + + Called after every successful save (``spec_create`` / ``write_manifest`` / + ``write_spec_markdown`` / ``write_spec_manifest_yaml``); reads back the + just-persisted ``spec.md``/``manifest.yaml`` (always in sync post-save) + rather than re-rendering them independently, so the recorded snapshot is + byte-identical to what a reader of the engine sees right now. + """ + spec_id = spec_id_for_key(manifest.id) + spec_md = engine.read_spec_md(spec_id) + manifest_yaml = engine.read_manifest_yaml(spec_id) + return spec_version_service.record_version( + db, + workspace_id=principal.workspace_id, + manifest=manifest, + spec_md=spec_md, + manifest_yaml=manifest_yaml, + created_by=principal.user_id, + ) + + class ConstitutionInitRequest(BaseModel): """Body for ``POST /spec/constitution``.""" @@ -131,6 +172,21 @@ class SpecCreateRequest(BaseModel): requirements: list[Requirement] = Field(default_factory=list) +class TextContent(BaseModel): + """Body for the ``spec.md`` / ``manifest.yaml`` write endpoints.""" + + content: str + + +class DraftSpecRequest(BaseModel): + """Body for ``POST /spec/draft`` (BYOK AI spec drafting; draft-only).""" + + goal: str = Field(min_length=1, description="One-line engineering goal to draft a spec for.") + epic_id: uuid.UUID | None = None + #: Optional project whose constitution seeds the spec-authoring prompt. + project_id: uuid.UUID | None = None + + # --------------------------------------------------------------------------- # # Routes # # --------------------------------------------------------------------------- # @@ -153,9 +209,16 @@ def constitution_init(engine: EngineDep, request: ConstitutionInitRequest) -> Co status_code=status.HTTP_201_CREATED, dependencies=[WriteGate], ) -def spec_create(engine: EngineDep, request: SpecCreateRequest) -> SpecManifest: - """Create a draft spec for an epic.""" - return engine.spec_create(request.epic_id, request.name, request.requirements) +def spec_create( + engine: EngineDep, + request: SpecCreateRequest, + db: DbSession, + principal: Annotated[Principal, Depends(get_current_principal)], +) -> SpecManifest: + """Create a draft spec for an epic (recorded as version 1).""" + manifest = engine.spec_create(request.epic_id, request.name, request.requirements) + _record_version(engine, db, principal, manifest) + return manifest @router.get("/specs/{spec_id}", response_model=SpecManifest, dependencies=[ReadGate]) @@ -166,10 +229,97 @@ def read_manifest(engine: EngineDep, spec_id: uuid.UUID) -> SpecManifest: @router.put("/specs/{spec_id}", response_model=SpecManifest, dependencies=[WriteGate]) -def write_manifest(engine: EngineDep, spec_id: uuid.UUID, manifest: SpecManifest) -> SpecManifest: - """Persist (create or update) a spec manifest.""" +def write_manifest( + engine: EngineDep, + spec_id: uuid.UUID, + manifest: SpecManifest, + db: DbSession, + principal: Annotated[Principal, Depends(get_current_principal)], +) -> SpecManifest: + """Persist (create or update) a spec manifest; records a new version.""" + with _spec_errors(): + updated = engine.write_manifest(manifest) + _record_version(engine, db, principal, updated) + return updated + + +@router.get( + "/specs/{spec_id}/markdown", + dependencies=[ReadGate], + response_class=PlainTextResponse, +) +def read_spec_markdown(engine: EngineDep, spec_id: uuid.UUID) -> PlainTextResponse: + """Read the spec's ``spec.md`` prose serialization (always kept in sync).""" with _spec_errors(): - return engine.write_manifest(manifest) + text = engine.read_spec_md(spec_id) + return PlainTextResponse(text) + + +@router.put("/specs/{spec_id}/markdown", response_model=SpecManifest, dependencies=[WriteGate]) +def write_spec_markdown( + engine: EngineDep, + spec_id: uuid.UUID, + body: TextContent, + db: DbSession, + principal: Annotated[Principal, Depends(get_current_principal)], +) -> SpecManifest: + """Save a spec edited as ``spec.md`` prose; re-renders ``manifest.yaml`` to match. + + The spec being edited must already exist at ``spec_id`` (404 otherwise); + the document's own frontmatter id governs which spec is written, mirroring + ``PUT /spec/specs/{spec_id}``. Records a new version on success. + """ + with _spec_errors(): + engine.read_manifest(spec_id) + updated = engine.save_spec_md(body.content) + _record_version(engine, db, principal, updated) + return updated + + +@router.get( + "/specs/{spec_id}/manifest", + dependencies=[ReadGate], + response_class=PlainTextResponse, +) +def read_spec_manifest_yaml(engine: EngineDep, spec_id: uuid.UUID) -> PlainTextResponse: + """Read the spec's ``manifest.yaml`` serialization (always kept in sync).""" + with _spec_errors(): + text = engine.read_manifest_yaml(spec_id) + return PlainTextResponse(text) + + +@router.put("/specs/{spec_id}/manifest", response_model=SpecManifest, dependencies=[WriteGate]) +def write_spec_manifest_yaml( + engine: EngineDep, + spec_id: uuid.UUID, + body: TextContent, + db: DbSession, + principal: Annotated[Principal, Depends(get_current_principal)], +) -> SpecManifest: + """Save a spec edited as ``manifest.yaml``; re-renders ``spec.md`` to match. + + Unlike the markdown endpoint, this may also *create* a new spec: when no + spec resolves to ``spec_id`` yet, the YAML's own id governs where it is + written (mirroring ``PUT /spec/specs/{spec_id}``'s create-or-update + semantics). Records a new version on success. + """ + with _spec_errors(): + updated = engine.save_manifest_yaml(body.content) + _record_version(engine, db, principal, updated) + return updated + + +@router.get( + "/constitution/{project_id}", + response_model=Constitution, + dependencies=[ReadGate], +) +def read_constitution(engine: EngineDep, project_id: uuid.UUID) -> Constitution: + """Read a project's constitution; 404 if it was never initialised.""" + constitution = engine.read_constitution(project_id) + if constitution is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="constitution not found") + return constitution @router.post("/specs/{spec_id}/clarify", response_model=SpecManifest, dependencies=[WriteGate]) @@ -211,6 +361,237 @@ def validate(engine: EngineDep, task_id: uuid.UUID) -> ValidationReport: return engine.validate(task_id) +# --------------------------------------------------------------------------- # +# ss-versioning: spec version history + diff # +# --------------------------------------------------------------------------- # +# +# A version is recorded (see ``_record_version``) on every save through the +# editing endpoints above. These read-only routes list a spec's version +# history and diff any two of its versions — both the raw ``spec.md`` prose +# (line-level) and the structured manifest (id-keyed adds/removes/changes per +# list field). Versions are looked up by ``spec_id`` (the same deterministic +# uuid as everywhere else in this router) + a 1-based ``version_number``. + + +class SpecVersionSummary(BaseModel): + """One row of a spec's version history (no snapshot payload).""" + + version_number: int + name: str + status: str + created_at: str + created_by: uuid.UUID | None = None + + +class SpecVersionDetail(SpecVersionSummary): + """A single version's full snapshot.""" + + manifest: SpecManifest + spec_md: str + manifest_yaml: str + + +class SpecVersionDiff(BaseModel): + """The diff between two versions of a spec.""" + + from_version: int + to_version: int + markdown: list[Any] = Field(default_factory=list) + manifest: ManifestDiff + + +def _version_summary(version: SpecVersion) -> SpecVersionSummary: + return SpecVersionSummary( + version_number=version.version_number, + name=version.name, + status=version.status, + created_at=version.created_at.isoformat(), + created_by=version.created_by, + ) + + +@router.get( + "/specs/{spec_id}/versions", + response_model=list[SpecVersionSummary], + dependencies=[ReadGate], +) +def list_spec_versions( + spec_id: uuid.UUID, + principal: Annotated[Principal, Depends(get_current_principal)], + db: DbSession, +) -> list[SpecVersionSummary]: + """List a spec's versions, newest first (empty if never saved).""" + versions = spec_version_service.list_versions( + db, workspace_id=principal.workspace_id, spec_id=spec_id + ) + return [_version_summary(v) for v in versions] + + +def _get_version_or_404( + db: DbSession, principal: Principal, spec_id: uuid.UUID, version_number: int +) -> SpecVersion: + version = spec_version_service.get_version( + db, workspace_id=principal.workspace_id, spec_id=spec_id, version_number=version_number + ) + if version is None: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"spec version {version_number} not found", + ) + return version + + +@router.get( + "/specs/{spec_id}/versions/{version_number}", + response_model=SpecVersionDetail, + dependencies=[ReadGate], +) +def read_spec_version( + spec_id: uuid.UUID, + version_number: int, + principal: Annotated[Principal, Depends(get_current_principal)], + db: DbSession, +) -> SpecVersionDetail: + """Read one version's full snapshot (manifest + both serializations).""" + version = _get_version_or_404(db, principal, spec_id, version_number) + return SpecVersionDetail( + **_version_summary(version).model_dump(), + manifest=SpecManifest.model_validate(version.manifest), + spec_md=version.spec_md, + manifest_yaml=version.manifest_yaml, + ) + + +@router.get( + "/specs/{spec_id}/versions/{from_version}/diff/{to_version}", + response_model=SpecVersionDiff, + dependencies=[ReadGate], +) +def diff_spec_versions( + spec_id: uuid.UUID, + from_version: int, + to_version: int, + principal: Annotated[Principal, Depends(get_current_principal)], + db: DbSession, +) -> SpecVersionDiff: + """Diff two versions of a spec: line-level markdown + structured manifest.""" + older = _get_version_or_404(db, principal, spec_id, from_version) + newer = _get_version_or_404(db, principal, spec_id, to_version) + markdown_diff = diff_markdown(older.spec_md, newer.spec_md) + manifest_diff = diff_manifest( + SpecManifest.model_validate(older.manifest), SpecManifest.model_validate(newer.manifest) + ) + return SpecVersionDiff( + from_version=from_version, + to_version=to_version, + markdown=[line.model_dump() for line in markdown_diff], + manifest=manifest_diff, + ) + + +# --------------------------------------------------------------------------- # +# ss-draft: BYOK AI spec drafting (POST /spec/draft) # +# --------------------------------------------------------------------------- # +# +# Uses the ao-model-router to pick a model for the workspace's BYOK provider, +# resolves the HARD-02 ModelClient (env/vault key) bound to that model, and +# streams a spec.md draft seeded with the project constitution. Draft-only: +# nothing is persisted. The binding is a single overridable dependency so tests +# inject a mocked ModelClient (no live key / network). + +#: The Adaptive Orchestration tier used for spec authoring. Drafting a spec is +#: high-leverage work, so it routes to the senior model by default. +_DRAFT_TIER: Tier = "senior" + + +@dataclass(frozen=True) +class DraftModelBinding: + """A resolved model client + the router-chosen model for a draft call.""" + + client: ModelClient + model: str + + +def get_draft_binding( + principal: Annotated[Principal, Depends(get_current_principal)], +) -> DraftModelBinding: + """Resolve the BYOK model client + router-chosen model for spec drafting. + + The provider comes from the workspace's ``FORGE_MODEL_*`` env config; the + ``ao-model-router`` maps the spec-authoring tier to a concrete model on that + provider; the HARD-02 client is then resolved with the workspace's BYOK key + bound to that model. Overridden in tests to inject a mocked client. + """ + from forge_agent.providers import ModelClientConfig, ModelClientError + from forge_agent.providers.router import ModelRouter + from forge_api.auth.service import get_auth_service + + config = ModelClientConfig.from_env() + if config is None: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail=( + "no model provider configured for spec drafting; set FORGE_MODEL_PROVIDER " + "and a BYOK key" + ), + ) + model = ModelRouter(provider=config.provider).resolve(_DRAFT_TIER) + try: + client = get_auth_service().resolve_model_client(principal.workspace_id, model=model) + except ModelClientError as exc: + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, detail=str(exc) + ) from exc + return DraftModelBinding(client=client, model=model) + + +DraftBindingDep = Annotated[DraftModelBinding, Depends(get_draft_binding)] + + +@router.post("/draft", response_model=SpecDraft, dependencies=[WriteGate]) +def draft_spec_endpoint( + engine: EngineDep, binding: DraftBindingDep, request: DraftSpecRequest +) -> SpecDraft: + """Draft a ``spec.md`` from a one-line goal via the BYOK model (draft-only). + + Seeds the spec-authoring prompt with the project constitution (when a + ``project_id`` resolving to one is supplied), streams the draft, and returns + a parsed :class:`SpecManifest` preview plus token/cost accounting. Nothing is + persisted — a human refines the draft via the spec-editing endpoints. + """ + from forge_agent.providers import ModelClientError + + constitution = ( + engine.read_constitution(request.project_id) if request.project_id is not None else None + ) + try: + return draft_spec( + binding.client, + goal=request.goal, + model=binding.model, + constitution=constitution, + epic_id=request.epic_id, + ) + except ModelClientError as exc: + raise HTTPException(status_code=status.HTTP_502_BAD_GATEWAY, detail=str(exc)) from exc + + +# --------------------------------------------------------------------------- # +# ss-import: external spec import (POST /spec/import) # +# --------------------------------------------------------------------------- # +# +# Turns an existing markdown or YAML spec (pasted or uploaded from outside +# Forge) into a spec.md draft — parse/normalize only, no model call. Draft-only +# like ``POST /spec/draft``: nothing is persisted; a human refines the result +# via the normal spec-editing endpoints. + + +@router.post("/import", response_model=SpecImport, dependencies=[WriteGate]) +def import_spec_endpoint(request: SpecImportRequest) -> SpecImport: + """Import an external markdown/YAML spec as a ``spec.md`` draft (draft-only).""" + return import_spec(request.content, source_format=request.source_format) + + # --------------------------------------------------------------------------- # # F23 spec-validation dashboard: GET /projects/{project_id}/specs # # --------------------------------------------------------------------------- # @@ -290,7 +671,13 @@ def project_spec_overview( "SpecCreateRequest", "SpecDashboard", "SpecEngineRegistry", + "SpecImport", + "SpecImportRequest", "SpecOverview", + "SpecVersionDetail", + "SpecVersionDiff", + "SpecVersionSummary", + "TextContent", "get_spec_engine", "get_spec_registry", "project_router", diff --git a/apps/api/forge_api/schemas/ao_settings.py b/apps/api/forge_api/schemas/ao_settings.py index ac2ec76f..895cddaf 100644 --- a/apps/api/forge_api/schemas/ao_settings.py +++ b/apps/api/forge_api/schemas/ao_settings.py @@ -8,11 +8,11 @@ from uuid import UUID -from forge_orchestration_policy import Strategy, Tier from pydantic import BaseModel, Field from forge_agent.providers.config import ProviderName from forge_contracts.orchestration_config import AgentRole, Effort, RoleConfigSource +from forge_orchestration_policy import Strategy, Tier __all__ = [ "AoSettingsOut", diff --git a/apps/api/forge_api/services/ao_settings_service.py b/apps/api/forge_api/services/ao_settings_service.py index 6365eca4..896a4906 100644 --- a/apps/api/forge_api/services/ao_settings_service.py +++ b/apps/api/forge_api/services/ao_settings_service.py @@ -15,16 +15,6 @@ from dataclasses import dataclass, field from uuid import UUID -from forge_orchestration_policy import Strategy, Tier -from forge_orchestration_policy.complexity import _JUNIOR_MAX as _DEFAULT_JUNIOR_MAX -from forge_orchestration_policy.complexity import _MEDIOR_MAX as _DEFAULT_MEDIOR_MAX -from forge_orchestration_policy.complexity import ( - BlastRadiusLevel, - SizingSignals, - score_complexity, -) -from forge_orchestration_policy.role_config import resolve_effective_config - from forge_agent.providers.config import ProviderName from forge_agent.providers.router import ModelRouter from forge_contracts.enums import Priority, TaskKind @@ -36,6 +26,15 @@ Effort, RoleConfigStore, ) +from forge_orchestration_policy import Strategy, Tier +from forge_orchestration_policy.complexity import _JUNIOR_MAX as _DEFAULT_JUNIOR_MAX +from forge_orchestration_policy.complexity import _MEDIOR_MAX as _DEFAULT_MEDIOR_MAX +from forge_orchestration_policy.complexity import ( + BlastRadiusLevel, + SizingSignals, + score_complexity, +) +from forge_orchestration_policy.role_config import resolve_effective_config __all__ = ["AoSettingsService", "EffectiveAoSettings", "RoutingPreview"] diff --git a/apps/api/forge_api/services/spec_draft_service.py b/apps/api/forge_api/services/spec_draft_service.py new file mode 100644 index 00000000..d8b791f2 --- /dev/null +++ b/apps/api/forge_api/services/spec_draft_service.py @@ -0,0 +1,231 @@ +"""BYOK AI spec drafting (slice ``ss-draft`` — track: Spec Studio). + +``POST /spec/draft`` asks the workspace's BYOK model — chosen by the +``ao-model-router`` and resolved through the existing HARD-02 +:class:`~forge_contracts.ModelClient` — to draft a ``spec.md`` from a one-line +goal, with a spec-authoring system prompt **seeded with the project +constitution**. The draft is *streamed* (progressive assembly), then parsed to a +:class:`~forge_contracts.SpecManifest` *preview*. This is draft-only: nothing is +persisted; a human refines the result via the normal spec-editing endpoints. + +Token/cost accounting rides the existing HARD-02 seam +(:class:`~forge_agent.providers.UsageAccumulator` + ``cost_usd``). The frozen +:class:`~forge_contracts.ModelStreamEvent` carries only text deltas (no usage), +so token counts for a streamed draft are *estimated* from the prompt and the +assembled draft and then priced through the very same cost table — the service +never reimplements the pricing logic. +""" + +from __future__ import annotations + +import math +import uuid +from typing import Any + +from pydantic import BaseModel, Field + +from forge_agent.providers import UsageAccumulator +from forge_contracts import ( + Constitution, + ModelMessage, + ModelRequest, + SpecManifest, + TokenUsage, +) +from forge_spec import SpecParseError, parse_spec_md + +__all__ = [ + "DRAFT_PLACEHOLDER_ID", + "SpecDraft", + "build_draft_request", + "build_system_prompt", + "draft_spec", + "estimate_tokens", +] + +#: A draft has no real spec id yet (it is never persisted), so the model is told +#: to use this placeholder in the frontmatter; a human assigns the real id when +#: the draft is created for real via ``POST /spec/specs`` / ``PUT`` editing. +DRAFT_PLACEHOLDER_ID = "SPEC-DRAFT" + +#: Default draft generation knobs (the injected client still owns provider-level +#: timeouts/retries; these only shape the request). +_DRAFT_MAX_TOKENS = 4000 +_DRAFT_TEMPERATURE = 0.2 + +_BASE_INSTRUCTIONS = ( + "You are Forge's spec author. You turn a one-line engineering goal into a " + "single, precise, testable specification following Spec-Driven Development. " + "Write concrete, verifiable requirements and Given/When/Then acceptance " + "criteria that each trace back to a requirement. Surface genuine ambiguity " + "as open questions rather than inventing scope. Output ONLY the spec.md " + "document — no preamble, no commentary, no code fences." +) + +#: The exact ``spec.md`` serialization contract the parser +#: (:func:`forge_spec.parse_spec_md`) expects. Kept in lock-step with +#: :func:`forge_spec.render_spec_md`. +_SPEC_MD_CONTRACT = ( + "Emit the document in EXACTLY this format:\n\n" + "---\n" + f"id: {DRAFT_PLACEHOLDER_ID}\n" + "status: draft\n" + "---\n\n" + "## Goal\n\n" + "\n\n" + "## Requirements\n\n" + "- **R1**: \n" + "- **R2**: \n\n" + "## Acceptance Criteria\n\n" + "- **A1** (R1): Given , when , then \n\n" + "## Constraints\n\n" + "- \n\n" + "## Open Questions\n\n" + "- **Q1**: \n\n" + "The YAML frontmatter block (between the '---' lines) and the '## Goal' " + "section are mandatory; omit any other section that has no content." +) + + +class SpecDraft(BaseModel): + """The draft-only result of ``POST /spec/draft`` (nothing is persisted).""" + + goal: str + epic_id: uuid.UUID | None = None + model: str + spec_md: str + #: The parsed preview, or ``None`` when the drafted markdown did not parse + #: (``parse_error`` then explains why — the raw ``spec_md`` is still returned + #: for the human to fix). + manifest: SpecManifest | None = None + parse_error: str | None = None + #: The ``model_usage`` accounting artifact (input/output tokens + ``cost_usd``). + usage: dict[str, Any] = Field(default_factory=dict) + + +def build_system_prompt(constitution: Constitution | None) -> str: + """Build the spec-authoring system prompt, seeded with the constitution. + + When a ``constitution`` is available its principles and architecture + guardrails are injected so the drafted spec conforms to the project's + engineering constitution; otherwise the base authoring instructions and the + ``spec.md`` format contract are used unchanged. + """ + parts: list[str] = [_BASE_INSTRUCTIONS] + if constitution is not None: + if constitution.principles: + bullet = "\n".join(f"- {p}" for p in constitution.principles) + parts.append("Project constitution — principles:\n" + bullet) + if constitution.architecture_guardrails: + bullet = "\n".join(f"- {g}" for g in constitution.architecture_guardrails) + parts.append("Project constitution — architecture guardrails:\n" + bullet) + parts.append(_SPEC_MD_CONTRACT) + return "\n\n".join(parts) + + +def build_draft_request( + *, + goal: str, + model: str, + system: str, + epic_id: uuid.UUID | None = None, +) -> ModelRequest: + """Build the streaming :class:`~forge_contracts.ModelRequest` for a draft.""" + user = f"Draft a spec.md for this engineering goal:\n\n{goal.strip()}" + if epic_id is not None: + user += f"\n\nThis spec belongs to epic {epic_id}." + return ModelRequest( + model=model, + system=system, + messages=[ModelMessage(role="user", content=user)], + max_tokens=_DRAFT_MAX_TOKENS, + temperature=_DRAFT_TEMPERATURE, + ) + + +def estimate_tokens(text: str) -> int: + """Estimate tokens for ``text`` (~4 chars/token; a non-empty string is >= 1). + + Used only because the streaming contract surfaces no provider usage; the + estimate is priced through the shared ``cost_usd`` table so accounting stays + consistent with the rest of the platform. + """ + if not text: + return 0 + return max(1, math.ceil(len(text) / 4)) + + +def _extract_spec_md(raw: str) -> str: + """Best-effort clean-up of the streamed text into a parseable ``spec.md``. + + Strips an accidental Markdown code-fence wrapper and any prose the model + emitted before the YAML frontmatter, so a slightly chatty model still yields + a parseable document. A well-formed draft passes through unchanged. + """ + text = raw.strip() + if not text: + return text + if text.startswith("```"): + lines = text.splitlines() + lines = lines[1:] # drop the opening ``` / ```markdown fence + if lines and lines[-1].strip() == "```": + lines = lines[:-1] + text = "\n".join(lines).strip() + marker = text.find("---") + if marker > 0: + text = text[marker:].strip() + return text + "\n" + + +def draft_spec( + client: Any, + *, + goal: str, + model: str, + constitution: Constitution | None = None, + epic_id: uuid.UUID | None = None, +) -> SpecDraft: + """Stream a spec draft from ``client`` and return the parsed preview + cost. + + ``client`` is any :class:`~forge_contracts.ModelClient` (a live BYOK client + in production, a mock in tests). The draft is assembled from the streamed + text deltas, parsed to a :class:`~forge_contracts.SpecManifest` preview + (never persisted), and token/cost is recorded via + :class:`~forge_agent.providers.UsageAccumulator`. + """ + system = build_system_prompt(constitution) + request = build_draft_request(goal=goal, model=model, system=system, epic_id=epic_id) + + chunks: list[str] = [] + for event in client.stream(request): + piece = event.delta if event.delta is not None else event.text + if piece: + chunks.append(piece) + raw = "".join(chunks) + + spec_md = _extract_spec_md(raw) + manifest: SpecManifest | None = None + parse_error: str | None = None + try: + manifest = parse_spec_md(spec_md) + except SpecParseError as exc: + parse_error = str(exc) + + prompt_text = (system or "") + "\n" + (request.messages[-1].content if request.messages else "") + accumulator = UsageAccumulator() + accumulator.add( + TokenUsage( + input_tokens=estimate_tokens(prompt_text), + output_tokens=estimate_tokens(raw), + ) + ) + + return SpecDraft( + goal=goal, + epic_id=epic_id, + model=model, + spec_md=spec_md, + manifest=manifest, + parse_error=parse_error, + usage=accumulator.to_artifact(model), + ) diff --git a/apps/api/forge_api/services/spec_import_service.py b/apps/api/forge_api/services/spec_import_service.py new file mode 100644 index 00000000..97e09915 --- /dev/null +++ b/apps/api/forge_api/services/spec_import_service.py @@ -0,0 +1,367 @@ +"""External spec import (slice ``ss-import`` — track: Spec Studio). + +Turns an existing spec authored *outside* Forge — a pasted/uploaded markdown +document (a GitHub issue, an RFC, a PRD) or a YAML manifest from another tool — +into a Forge ``spec.md`` draft, so a human can bring existing work into the SDD +lifecycle instead of retyping it. + +Three tiers of effort, cheapest first: + +1. **Direct parse** — the content is already a valid Forge ``spec.md`` + (:func:`forge_spec.parse_spec_md`) or ``manifest.yaml`` + (:func:`forge_spec.load_manifest`); it round-trips byte-for-byte in meaning. +2. **Normalize** — the content is YAML or markdown but uses looser shapes + (``title``/``summary`` instead of ``name``, plain string lists instead of + typed requirement objects, arbitrary heading names). Best-effort mapping + onto :class:`~forge_contracts.SpecManifest`, assigning sequential ids + (``R1``, ``A1``, ``Q1``, ...) where the source had none. +3. **Give up gracefully** — genuinely unparseable content (e.g. binary noise) + is still returned verbatim as ``spec_md`` with ``parse_error`` set, mirroring + ``ss-draft``'s graceful-failure contract, so the human can hand-fix it in the + markdown editor rather than losing the paste. + +This is draft-only, like ``POST /spec/draft``: nothing is persisted here — the +human reviews/refines the result and saves it via the normal spec-editing +endpoints (``PUT /spec/specs/{id}`` / ``/markdown`` / ``/manifest``). +""" + +from __future__ import annotations + +import re +from typing import Any, Literal + +import yaml +from pydantic import BaseModel, Field + +from forge_contracts import ( + AcceptanceCriterion, + OpenQuestion, + Requirement, + SpecManifest, +) +from forge_contracts.enums import SpecStatus +from forge_spec import SpecParseError, load_manifest, parse_spec_md, render_spec_md + +__all__ = [ + "IMPORT_PLACEHOLDER_ID", + "SpecImport", + "SpecImportFormat", + "detect_format", + "import_spec", +] + +#: An imported spec has no real spec id yet (it is never persisted directly), +#: mirroring ``ss-draft``'s ``DRAFT_PLACEHOLDER_ID`` convention. +IMPORT_PLACEHOLDER_ID = "SPEC-IMPORT" + +SpecImportFormat = Literal["markdown", "yaml"] + +_REQUESTED_FORMATS = ("markdown", "yaml", "auto") + + +class SpecImport(BaseModel): + """The draft-only result of ``POST /spec/import`` (nothing is persisted).""" + + source_format: SpecImportFormat + spec_md: str + #: The parsed/normalized preview, or ``None`` when the content did not + #: parse or normalize (``parse_error`` then explains why). + manifest: SpecManifest | None = None + parse_error: str | None = None + #: ``True`` when the source needed best-effort normalization (loose YAML + #: keys, arbitrary markdown headings) rather than parsing directly as a + #: canonical Forge ``spec.md`` / ``manifest.yaml``. + normalized: bool = False + + +# --------------------------------------------------------------------------- # +# Format detection # +# --------------------------------------------------------------------------- # + + +def detect_format(content: str, requested: str = "auto") -> SpecImportFormat: + """Resolve the source format: an explicit hint, or sniffed from ``content``. + + A canonical (or loosely-shaped) ``spec.md`` always has at least one + Markdown ``#`` heading; genuine YAML never does, so heading detection is + the deciding signal. Content that is neither valid YAML nor has headings + still falls back to markdown (the more forgiving of the two normalizers). + """ + if requested in ("markdown", "yaml"): + return requested # type: ignore[return-value] + stripped = content.strip() + if _HEADING_RE.search(stripped) is not None: + return "markdown" + try: + data = yaml.safe_load(stripped) + except yaml.YAMLError: + return "markdown" + if isinstance(data, dict) and data: + return "yaml" + return "markdown" + + +# --------------------------------------------------------------------------- # +# Loose-YAML normalization # +# --------------------------------------------------------------------------- # + +#: Alternate keys another tool might use for each manifest field, tried in order. +_YAML_NAME_KEYS = ("name", "title", "summary") +_YAML_REQUIREMENT_KEYS = ("requirements", "user_stories", "stories") +_YAML_ACCEPTANCE_KEYS = ("acceptance_criteria", "acceptance", "criteria") +_YAML_CONSTRAINT_KEYS = ("constraints", "non_functional_requirements", "nfrs") +_YAML_QUESTION_KEYS = ("open_questions", "questions") + + +def _first_present(data: dict[str, Any], keys: tuple[str, ...]) -> Any: + for key in keys: + value = data.get(key) + if value: + return value + return None + + +def _text_of(item: Any) -> str: + if isinstance(item, str): + return item + if isinstance(item, dict): + for key in ("text", "description", "body", "summary"): + value = item.get(key) + if isinstance(value, str) and value: + return value + return str(item) + + +def _coerce_requirements(raw: Any) -> list[Requirement]: + if not isinstance(raw, list): + return [] + out: list[Requirement] = [] + for i, item in enumerate(raw, start=1): + rid = item.get("id") if isinstance(item, dict) else None + out.append(Requirement(id=str(rid) if rid else f"R{i}", text=_text_of(item))) + return out + + +def _coerce_acceptance(raw: Any, requirement_ids: list[str]) -> list[AcceptanceCriterion]: + if not isinstance(raw, list): + return [] + out: list[AcceptanceCriterion] = [] + for i, item in enumerate(raw, start=1): + aid = item.get("id") if isinstance(item, dict) else None + refs = item.get("req_refs") if isinstance(item, dict) else None + out.append( + AcceptanceCriterion( + id=str(aid) if aid else f"A{i}", + text=_text_of(item), + req_refs=list(refs) if isinstance(refs, list) else requirement_ids, + ) + ) + return out + + +def _coerce_open_questions(raw: Any) -> list[OpenQuestion]: + if not isinstance(raw, list): + return [] + out: list[OpenQuestion] = [] + for i, item in enumerate(raw, start=1): + qid = item.get("id") if isinstance(item, dict) else None + resolution = item.get("resolution") if isinstance(item, dict) else None + out.append( + OpenQuestion( + id=str(qid) if qid else f"Q{i}", + text=_text_of(item), + resolution=resolution if isinstance(resolution, str) else None, + ) + ) + return out + + +def _coerce_str_list(raw: Any) -> list[str]: + if not isinstance(raw, list): + return [] + return [_text_of(item) for item in raw] + + +def _manifest_from_loose_yaml(data: dict[str, Any]) -> SpecManifest: + """Best-effort map a loosely-shaped YAML mapping onto ``SpecManifest``.""" + name = _first_present(data, _YAML_NAME_KEYS) or "Imported spec" + requirements = _coerce_requirements(_first_present(data, _YAML_REQUIREMENT_KEYS)) + acceptance = _coerce_acceptance( + _first_present(data, _YAML_ACCEPTANCE_KEYS), [r.id for r in requirements] + ) + constraints = _coerce_str_list(_first_present(data, _YAML_CONSTRAINT_KEYS)) + open_questions = _coerce_open_questions(_first_present(data, _YAML_QUESTION_KEYS)) + return SpecManifest( + id=IMPORT_PLACEHOLDER_ID, + name=str(name), + status=SpecStatus.DRAFT, + requirements=requirements, + acceptance_criteria=acceptance, + constraints=constraints, + open_questions=open_questions, + ) + + +# --------------------------------------------------------------------------- # +# Loose-markdown normalization # +# --------------------------------------------------------------------------- # + +# Linear-time patterns: a greedy `(.+)` to end-of-line (no lazy `.+?` + trailing +# `\s*$` overlap, which CodeQL flags as polynomial/ReDoS on user-supplied text), +# with `[ \t]` separators so whitespace classes don't overlap the capture. The +# callers already `.strip()` the captured group, so trailing spaces are handled. +_HEADING_RE = re.compile(r"^(#{1,6})[ \t]+(\S.*)$", re.MULTILINE) +_BULLET_RE = re.compile(r"^[ \t]*[-*][ \t]+(\S.*)$") + +#: Heading text (lowercased, trailing ':' stripped) -> the bucket it feeds. +_SECTION_ALIASES: dict[str, str] = { + "goal": "goal", + "summary": "goal", + "overview": "goal", + "objective": "goal", + "description": "goal", + "requirements": "requirements", + "functional requirements": "requirements", + "user stories": "requirements", + "acceptance criteria": "acceptance_criteria", + "acceptance": "acceptance_criteria", + "constraints": "constraints", + "non-functional requirements": "constraints", + "non functional requirements": "constraints", + "open questions": "open_questions", + "questions": "open_questions", +} + + +def _normalize_markdown(text: str) -> SpecManifest: + """Best-effort map arbitrary markdown headings/bullets onto ``SpecManifest``.""" + buckets: dict[str, list[str]] = { + "goal": [], + "requirements": [], + "acceptance_criteria": [], + "constraints": [], + "open_questions": [], + } + name: str | None = None + current: str | None = None + for line in text.splitlines(): + heading = _HEADING_RE.match(line) + if heading is not None: + level, title = heading.group(1), heading.group(2).strip().lower().rstrip(":") + key = _SECTION_ALIASES.get(title) + if key is not None: + current = key + elif level == "#" and name is None: + name = heading.group(2).strip() + current = None + else: + current = None + continue + if current is None: + continue + bullet = _BULLET_RE.match(line) + stripped = line.strip() + if bullet is not None: + buckets[current].append(bullet.group(1).strip()) + elif stripped: + buckets[current].append(stripped) + + if name is None: + if buckets["goal"]: + name = buckets["goal"][0] + else: + first_line = next((ln.strip() for ln in text.splitlines() if ln.strip()), "") + name = first_line[:200] or "Imported spec" + + requirements = [ + Requirement(id=f"R{i}", text=t) for i, t in enumerate(buckets["requirements"], 1) + ] + requirement_ids = [r.id for r in requirements] + acceptance = [ + AcceptanceCriterion(id=f"A{i}", text=t, req_refs=requirement_ids) + for i, t in enumerate(buckets["acceptance_criteria"], 1) + ] + open_questions = [ + OpenQuestion(id=f"Q{i}", text=t) for i, t in enumerate(buckets["open_questions"], 1) + ] + + return SpecManifest( + id=IMPORT_PLACEHOLDER_ID, + name=name, + status=SpecStatus.DRAFT, + requirements=requirements, + acceptance_criteria=acceptance, + constraints=buckets["constraints"], + open_questions=open_questions, + ) + + +# --------------------------------------------------------------------------- # +# Entry point # +# --------------------------------------------------------------------------- # + + +def import_spec(content: str, *, source_format: str = "auto") -> SpecImport: + """Import ``content`` (an external markdown or YAML spec) as a draft. + + ``source_format`` is ``"markdown"``, ``"yaml"``, or ``"auto"`` (sniffed via + :func:`detect_format`). Always returns a result — never raises — mirroring + ``ss-draft``'s graceful-failure contract: unparseable content still comes + back with the raw ``spec_md`` and a ``parse_error`` explaining why. + """ + fmt = detect_format(content, source_format) + + if fmt == "yaml": + try: + manifest = load_manifest(content) + return SpecImport( + source_format="yaml", + spec_md=render_spec_md(manifest), + manifest=manifest, + normalized=False, + ) + except Exception: + pass + try: + data = yaml.safe_load(content) or {} + if not isinstance(data, dict): + raise ValueError("YAML content must deserialize to a mapping") + manifest = _manifest_from_loose_yaml(data) + return SpecImport( + source_format="yaml", + spec_md=render_spec_md(manifest), + manifest=manifest, + normalized=True, + ) + except Exception as exc: + return SpecImport(source_format="yaml", spec_md=content, parse_error=str(exc)) + + try: + manifest = parse_spec_md(content) + return SpecImport( + source_format="markdown", spec_md=content, manifest=manifest, normalized=False + ) + except SpecParseError: + pass + try: + manifest = _normalize_markdown(content) + return SpecImport( + source_format="markdown", + spec_md=render_spec_md(manifest), + manifest=manifest, + normalized=True, + ) + except Exception as exc: + return SpecImport( + source_format="markdown", spec_md=content, parse_error=str(exc), normalized=True + ) + + +class SpecImportRequest(BaseModel): + """Body for ``POST /spec/import``.""" + + content: str = Field(min_length=1, description="The pasted/uploaded spec text.") + source_format: Literal["markdown", "yaml", "auto"] = "auto" + + +__all__.append("SpecImportRequest") diff --git a/apps/api/forge_api/services/spec_version_service.py b/apps/api/forge_api/services/spec_version_service.py new file mode 100644 index 00000000..e6f40e70 --- /dev/null +++ b/apps/api/forge_api/services/spec_version_service.py @@ -0,0 +1,88 @@ +"""Records + reads spec version snapshots (ss-versioning). + +``FileSpecEngine`` is filesystem-backed and keeps no history: every save +overwrites ``manifest.yaml``/``spec.md`` in place. This service is the durable +side-channel the spec router calls on every save (``spec_create``, +``write_manifest``, ``write_spec_markdown``, ``write_spec_manifest_yaml``): it +appends an immutable :class:`~forge_db.models.SpecVersion` row carrying a full +snapshot, so Spec Studio can list a spec's version history and diff any two +versions even though the engine itself only ever holds the *current* state. + +Workspace-scoped throughout (mirrors every other DB-backed repo in +``apps/api``): a version is only ever recorded, listed, or read for the +caller's own workspace. +""" + +from __future__ import annotations + +import uuid + +from sqlalchemy import func, select +from sqlalchemy.orm import Session + +from forge_contracts import SpecManifest +from forge_db.models import SpecVersion +from forge_spec import spec_id_for_key + + +def record_version( + db: Session, + *, + workspace_id: uuid.UUID, + manifest: SpecManifest, + spec_md: str, + manifest_yaml: str, + created_by: uuid.UUID | None, +) -> SpecVersion: + """Append the next version snapshot for ``manifest.id`` and commit it.""" + spec_id = spec_id_for_key(manifest.id) + next_number = ( + db.execute( + select(func.coalesce(func.max(SpecVersion.version_number), 0)).where( + SpecVersion.workspace_id == workspace_id, + SpecVersion.spec_id == spec_id, + ) + ).scalar_one() + + 1 + ) + version = SpecVersion( + workspace_id=workspace_id, + spec_id=spec_id, + spec_key=manifest.id, + version_number=next_number, + name=manifest.name, + status=manifest.status.value, + manifest=manifest.model_dump(mode="json"), + spec_md=spec_md, + manifest_yaml=manifest_yaml, + created_by=created_by, + ) + db.add(version) + db.commit() + db.refresh(version) + return version + + +def list_versions(db: Session, *, workspace_id: uuid.UUID, spec_id: uuid.UUID) -> list[SpecVersion]: + """List a spec's versions, newest first.""" + stmt = ( + select(SpecVersion) + .where(SpecVersion.workspace_id == workspace_id, SpecVersion.spec_id == spec_id) + .order_by(SpecVersion.version_number.desc()) + ) + return list(db.execute(stmt).scalars()) + + +def get_version( + db: Session, *, workspace_id: uuid.UUID, spec_id: uuid.UUID, version_number: int +) -> SpecVersion | None: + """Read one specific version snapshot, or ``None`` if unknown.""" + stmt = select(SpecVersion).where( + SpecVersion.workspace_id == workspace_id, + SpecVersion.spec_id == spec_id, + SpecVersion.version_number == version_number, + ) + return db.execute(stmt).scalar_one_or_none() + + +__all__ = ["get_version", "list_versions", "record_version"] diff --git a/apps/api/tests/test_rbac_tenant_r2.py b/apps/api/tests/test_rbac_tenant_r2.py index eefec7ad..591f12d3 100644 --- a/apps/api/tests/test_rbac_tenant_r2.py +++ b/apps/api/tests/test_rbac_tenant_r2.py @@ -245,13 +245,41 @@ def test_workflow_run_is_workspace_scoped() -> None: def test_spec_is_workspace_scoped(tmp_path) -> None: + from sqlalchemy import StaticPool, create_engine + from sqlalchemy.orm import Session, sessionmaker + + from forge_api.db import get_db from forge_api.routers.spec import SpecEngineRegistry, get_spec_registry + from forge_db.base import Base + from forge_db.models import Workspace from forge_spec import spec_id_for_key app = create_app() registry = SpecEngineRegistry(tmp_path / "specs") app.dependency_overrides[get_spec_registry] = lambda: registry + # ss-versioning: spec saves now also record a ``spec_version`` row, so + # this needs a real DB session (SQLite in-memory, mirroring the other + # hermetic spec-router test fixtures). + db_engine = create_engine( + "sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool + ) + Base.metadata.create_all(db_engine) + db_factory = sessionmaker(bind=db_engine, expire_on_commit=False, class_=Session) + with db_factory() as session: + session.add(Workspace(id=TEST_WORKSPACE_ID, name="Acme", slug="acme")) + session.add(Workspace(id=OTHER_WORKSPACE_ID, name="Other", slug="other")) + session.commit() + + def _override_db() -> Iterator[Session]: + session = db_factory() + try: + yield session + finally: + session.close() + + app.dependency_overrides[get_db] = _override_db + _as(app, make_test_principal(role=UserRole.MEMBER, workspace_id=TEST_WORKSPACE_ID)) with TestClient(app) as client: created = client.post( diff --git a/apps/api/tests/test_spec_draft.py b/apps/api/tests/test_spec_draft.py new file mode 100644 index 00000000..3f8767aa --- /dev/null +++ b/apps/api/tests/test_spec_draft.py @@ -0,0 +1,293 @@ +"""ss-draft: BYOK AI spec drafting (``POST /spec/draft``). + +Covers the service (prompt shape seeded with the constitution, streaming +assembly, parse to a manifest preview, token/cost accounting) and the wired +endpoint (draft-only, RBAC, constitution seeding). The ``ModelClient`` is +MOCKED throughout — no live key, no network. +""" + +from __future__ import annotations + +import uuid +from collections.abc import Callable, Iterator +from pathlib import Path + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from forge_agent.providers import cost_usd +from forge_api.deps import Principal +from forge_api.main import create_app +from forge_api.routers.spec import DraftModelBinding, get_draft_binding, get_spec_engine +from forge_api.services.spec_draft_service import ( + DRAFT_PLACEHOLDER_ID, + build_system_prompt, + draft_spec, + estimate_tokens, +) +from forge_contracts import ( + AcceptanceCriterion, + Constitution, + ModelRequest, + ModelResponse, + ModelStreamEvent, + Requirement, + SpecManifest, + TokenUsage, +) +from forge_contracts.enums import SpecStatus, UserRole +from forge_spec import FileSpecEngine, render_spec_md, spec_id_for_key + +_MODEL = "claude-opus-4-8" + + +def _principal(role: UserRole = UserRole.ADMIN) -> Principal: + return Principal( + user_id=uuid.uuid4(), + workspace_id=uuid.uuid4(), + role=role, + email="test@forge.local", + auth_method="test", + scopes=["*"], + ) + + +def _sample_spec_md() -> str: + """A well-formed spec.md the model would emit (guaranteed round-trippable).""" + manifest = SpecManifest( + id=DRAFT_PLACEHOLDER_ID, + name="Customer search by name", + status=SpecStatus.DRAFT, + requirements=[Requirement(id="R1", text="Search customers by name")], + acceptance_criteria=[ + AcceptanceCriterion( + id="A1", + text="Given a name, when searching, then matching customers are returned", + req_refs=["R1"], + ) + ], + ) + return render_spec_md(manifest) + + +class _StreamingSpy: + """A mocked ``ModelClient`` that streams ``chunks`` and records requests.""" + + def __init__(self, chunks: list[str]) -> None: + self._chunks = chunks + self.requests: list[ModelRequest] = [] + + def complete(self, request: ModelRequest) -> ModelResponse: # pragma: no cover - unused + self.requests.append(request) + return ModelResponse(content="".join(self._chunks)) + + def stream(self, request: ModelRequest) -> Iterator[ModelStreamEvent]: + self.requests.append(request) + for chunk in self._chunks: + yield ModelStreamEvent(type="text", text=chunk, delta=chunk) + + +def _chunked(text: str, size: int = 37) -> list[str]: + return [text[i : i + size] for i in range(0, len(text), size)] or [""] + + +# --------------------------------------------------------------------------- # +# Service unit tests # +# --------------------------------------------------------------------------- # + + +def test_estimate_tokens_is_deterministic_and_nonzero() -> None: + assert estimate_tokens("") == 0 + assert estimate_tokens("x") == 1 + assert estimate_tokens("abcd" * 10) == 10 + + +def test_build_system_prompt_seeds_constitution() -> None: + constitution = Constitution( + project_id=uuid.uuid4(), + principles=["Prefer boring technology", "Tests are non-negotiable"], + architecture_guardrails=["Singular table names", "tz-aware datetimes"], + ) + prompt = build_system_prompt(constitution) + assert "Prefer boring technology" in prompt + assert "Tests are non-negotiable" in prompt + assert "Singular table names" in prompt + # The spec.md format contract is always present so the draft parses. + assert "## Goal" in prompt + assert DRAFT_PLACEHOLDER_ID in prompt + + +def test_build_system_prompt_without_constitution() -> None: + prompt = build_system_prompt(None) + assert "## Goal" in prompt # the spec.md format contract is always present + assert DRAFT_PLACEHOLDER_ID in prompt + assert "constitution" not in prompt.lower() # none supplied -> not injected + + +def test_draft_spec_assembles_stream_and_parses() -> None: + spec_md = _sample_spec_md() + client = _StreamingSpy(_chunked(spec_md)) + + draft = draft_spec(client, goal="Let users search customers by name", model=_MODEL) + + # Streaming assembly reconstructed the full document across chunks. + assert draft.spec_md == spec_md + assert draft.parse_error is None + assert draft.manifest is not None + assert draft.manifest.name == "Customer search by name" + assert draft.manifest.requirements[0].id == "R1" + assert draft.model == _MODEL + + +def test_draft_spec_prompt_shape_carries_goal_and_constitution() -> None: + client = _StreamingSpy(_chunked(_sample_spec_md())) + constitution = Constitution(project_id=uuid.uuid4(), principles=["Ship small, safe changes"]) + epic_id = uuid.uuid4() + + draft_spec( + client, + goal="Add SSO login", + model=_MODEL, + constitution=constitution, + epic_id=epic_id, + ) + + assert len(client.requests) == 1 + request = client.requests[0] + assert request.model == _MODEL + assert request.system is not None and "Ship small, safe changes" in request.system + user = request.messages[-1].content + assert "Add SSO login" in user + assert str(epic_id) in user + + +def test_draft_spec_records_cost_via_pricing_table() -> None: + spec_md = _sample_spec_md() + client = _StreamingSpy(_chunked(spec_md)) + + draft = draft_spec(client, goal="Search customers", model=_MODEL) + + usage = draft.usage + assert usage["output_tokens"] == estimate_tokens(spec_md) + assert usage["input_tokens"] > 0 + assert usage["calls"] == 1 + # Cost rides the shared HARD-02 pricing table (not reimplemented here). + expected = cost_usd( + _MODEL, + TokenUsage(input_tokens=usage["input_tokens"], output_tokens=usage["output_tokens"]), + ) + assert usage["cost_usd"] == expected + assert usage["cost_usd"] > 0.0 + + +def test_draft_spec_parse_error_is_graceful() -> None: + client = _StreamingSpy(["This is not a spec at all, just prose."]) + + draft = draft_spec(client, goal="whatever", model=_MODEL) + + assert draft.manifest is None + assert draft.parse_error is not None + assert draft.spec_md # raw text still returned for the human to fix + assert draft.usage["cost_usd"] >= 0.0 + + +def test_draft_spec_strips_code_fence_wrapper() -> None: + spec_md = _sample_spec_md() + fenced = "```markdown\n" + spec_md + "```\n" + client = _StreamingSpy(_chunked(fenced)) + + draft = draft_spec(client, goal="Search customers", model=_MODEL) + + assert draft.manifest is not None + assert draft.manifest.name == "Customer search by name" + + +# --------------------------------------------------------------------------- # +# Endpoint integration tests # +# --------------------------------------------------------------------------- # + + +@pytest.fixture +def spy() -> _StreamingSpy: + return _StreamingSpy(_chunked(_sample_spec_md())) + + +def _make_client( + tmp_path: Path, + authenticate_app: Callable[..., FastAPI], + spy: _StreamingSpy, + *, + role: UserRole = UserRole.ADMIN, +) -> tuple[TestClient, FileSpecEngine]: + app = create_app() + authenticate_app(app, _principal(role=role)) + engine = FileSpecEngine(root=tmp_path / "specs") + app.dependency_overrides[get_spec_engine] = lambda: engine + app.dependency_overrides[get_draft_binding] = lambda: DraftModelBinding( + client=spy, model=_MODEL + ) + return TestClient(app), engine + + +def test_draft_endpoint_returns_manifest_preview( + tmp_path: Path, authenticate_app: Callable[..., FastAPI], spy: _StreamingSpy +) -> None: + client, _ = _make_client(tmp_path, authenticate_app, spy) + with client: + resp = client.post("/spec/draft", json={"goal": "Search customers by name"}) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["model"] == _MODEL + assert body["manifest"]["name"] == "Customer search by name" + assert body["parse_error"] is None + assert body["usage"]["cost_usd"] > 0.0 + assert body["spec_md"].startswith("---") + + +def test_draft_endpoint_seeds_constitution_from_project( + tmp_path: Path, authenticate_app: Callable[..., FastAPI], spy: _StreamingSpy +) -> None: + client, engine = _make_client(tmp_path, authenticate_app, spy) + project_id = uuid.uuid4() + engine.constitution_init(project_id, ["Latency budget is 200ms"]) + with client: + resp = client.post( + "/spec/draft", + json={"goal": "Add a caching layer", "project_id": str(project_id)}, + ) + assert resp.status_code == 200, resp.text + # The seeded constitution principle reached the model's system prompt. + assert spy.requests + assert "Latency budget is 200ms" in (spy.requests[0].system or "") + + +def test_draft_endpoint_requires_write_permission( + tmp_path: Path, authenticate_app: Callable[..., FastAPI], spy: _StreamingSpy +) -> None: + client, _ = _make_client(tmp_path, authenticate_app, spy, role=UserRole.VIEWER) + with client: + resp = client.post("/spec/draft", json={"goal": "Search customers"}) + assert resp.status_code == 403 + + +def test_draft_endpoint_rejects_empty_goal( + tmp_path: Path, authenticate_app: Callable[..., FastAPI], spy: _StreamingSpy +) -> None: + client, _ = _make_client(tmp_path, authenticate_app, spy) + with client: + resp = client.post("/spec/draft", json={"goal": ""}) + assert resp.status_code == 422 + + +def test_draft_endpoint_does_not_persist( + tmp_path: Path, authenticate_app: Callable[..., FastAPI], spy: _StreamingSpy +) -> None: + """Draft-only: the previewed spec is not written to the engine.""" + client, _ = _make_client(tmp_path, authenticate_app, spy) + with client: + resp = client.post("/spec/draft", json={"goal": "Search customers by name"}) + assert resp.status_code == 200 + spec_uuid = spec_id_for_key(DRAFT_PLACEHOLDER_ID) + fetched = client.get(f"/spec/specs/{spec_uuid}") + assert fetched.status_code == 404 diff --git a/apps/api/tests/test_spec_import.py b/apps/api/tests/test_spec_import.py new file mode 100644 index 00000000..b0bee163 --- /dev/null +++ b/apps/api/tests/test_spec_import.py @@ -0,0 +1,300 @@ +"""ss-import: external spec import (``POST /spec/import``). + +Covers the service (direct parse, loose-YAML normalization, loose-markdown +normalization, graceful failure) and the wired endpoint (RBAC, draft-only — +nothing persisted). No model client involved — this is parse/normalize only. +""" + +from __future__ import annotations + +import uuid +from collections.abc import Callable +from pathlib import Path + +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from forge_api.deps import Principal +from forge_api.main import create_app +from forge_api.routers.spec import get_spec_engine +from forge_api.services.spec_import_service import ( + IMPORT_PLACEHOLDER_ID, + detect_format, + import_spec, +) +from forge_contracts import Requirement, SpecManifest +from forge_contracts.enums import SpecStatus, UserRole +from forge_spec import FileSpecEngine, render_spec_md, spec_id_for_key + +# --------------------------------------------------------------------------- # +# Format detection # +# --------------------------------------------------------------------------- # + + +def test_detect_format_honors_explicit_hint() -> None: + assert detect_format("id: x\nname: y\n", "yaml") == "yaml" + assert detect_format("id: x\nname: y\n", "markdown") == "markdown" + + +def test_detect_format_sniffs_markdown_from_headings() -> None: + assert detect_format("# Title\n\nSome body text.") == "markdown" + + +def test_detect_format_sniffs_yaml_from_mapping() -> None: + assert detect_format("title: Thing\nrequirements:\n - do X\n") == "yaml" + + +def test_detect_format_falls_back_to_markdown_for_unparseable() -> None: + assert detect_format("not: [valid: yaml: at: all") == "markdown" + + +# --------------------------------------------------------------------------- # +# Tier 1: direct parse (already-canonical Forge documents) # +# --------------------------------------------------------------------------- # + + +def _canonical_spec_md() -> str: + manifest = SpecManifest( + id="SPEC-42", + name="Existing spec", + requirements=[Requirement(id="R1", text="Do the thing")], + ) + return render_spec_md(manifest) + + +def test_import_spec_md_that_is_already_canonical_passes_through() -> None: + spec_md = _canonical_spec_md() + + result = import_spec(spec_md) + + assert result.source_format == "markdown" + assert result.normalized is False + assert result.parse_error is None + assert result.manifest is not None + assert result.manifest.id == "SPEC-42" + assert result.spec_md == spec_md + + +def test_import_manifest_yaml_that_is_already_canonical_passes_through() -> None: + from forge_spec import dump_manifest + + manifest = SpecManifest(id="SPEC-7", name="Canonical yaml spec") + yaml_text = dump_manifest(manifest) + + result = import_spec(yaml_text, source_format="yaml") + + assert result.source_format == "yaml" + assert result.normalized is False + assert result.parse_error is None + assert result.manifest is not None + assert result.manifest.id == "SPEC-7" + assert result.manifest.name == "Canonical yaml spec" + + +# --------------------------------------------------------------------------- # +# Tier 2: normalize (loose shapes) # +# --------------------------------------------------------------------------- # + + +def test_import_loose_markdown_normalizes_sections() -> None: + content = ( + "# Customer search\n\n" + "## Requirements\n" + "- Search customers by name\n" + "- Filter by status\n\n" + "## Acceptance Criteria\n" + "- Given a name, when searching, then matches return\n\n" + "## Constraints\n" + "- Must respond within 200ms\n\n" + "## Open Questions\n" + "- Should archived customers be included?\n" + ) + + result = import_spec(content) + + assert result.source_format == "markdown" + assert result.normalized is True + assert result.parse_error is None + manifest = result.manifest + assert manifest is not None + assert manifest.name == "Customer search" + assert [r.text for r in manifest.requirements] == [ + "Search customers by name", + "Filter by status", + ] + assert manifest.requirements[0].id == "R1" + assert manifest.acceptance_criteria[0].req_refs == ["R1", "R2"] + assert manifest.constraints == ["Must respond within 200ms"] + assert manifest.open_questions[0].id == "Q1" + # The normalized preview re-renders as valid, round-trippable spec.md. + assert result.spec_md.startswith("---") + from forge_spec import parse_spec_md + + assert parse_spec_md(result.spec_md).name == "Customer search" + + +def test_import_loose_markdown_without_h1_falls_back_to_first_line() -> None: + content = "Just some free-form notes about a feature.\n\nNo headings at all here." + + result = import_spec(content) + + assert result.manifest is not None + assert result.manifest.name == "Just some free-form notes about a feature." + assert result.manifest.id == IMPORT_PLACEHOLDER_ID + + +def test_import_loose_yaml_normalizes_alternate_keys() -> None: + content = ( + "title: Customer search\n" + "requirements:\n" + " - Search customers by name\n" + " - Filter by status\n" + "acceptance:\n" + " - Given a name, when searching, then matches return\n" + "constraints:\n" + " - Must respond within 200ms\n" + ) + + result = import_spec(content, source_format="yaml") + + assert result.source_format == "yaml" + assert result.normalized is True + assert result.parse_error is None + manifest = result.manifest + assert manifest is not None + assert manifest.name == "Customer search" + assert len(manifest.requirements) == 2 + assert manifest.requirements[0].id == "R1" + assert manifest.acceptance_criteria[0].req_refs == ["R1", "R2"] + assert manifest.constraints == ["Must respond within 200ms"] + + +def test_import_loose_yaml_with_dict_items_extracts_text() -> None: + content = "name: Thing\nrequirements:\n - id: CUSTOM-1\n text: A dict-shaped requirement\n" + + result = import_spec(content, source_format="yaml") + + assert result.manifest is not None + assert result.manifest.requirements[0].id == "CUSTOM-1" + assert result.manifest.requirements[0].text == "A dict-shaped requirement" + + +def test_import_normalized_result_defaults_to_draft_status() -> None: + result = import_spec("# Some spec\n\n## Requirements\n- A thing\n") + assert result.manifest is not None + assert result.manifest.status == SpecStatus.DRAFT + + +# --------------------------------------------------------------------------- # +# Tier 3: graceful failure # +# --------------------------------------------------------------------------- # + + +def test_import_yaml_that_is_not_a_mapping_fails_gracefully() -> None: + result = import_spec("- just\n- a\n- list\n", source_format="yaml") + + assert result.manifest is None + assert result.parse_error is not None + assert result.spec_md # raw content preserved for the human to fix + + +def test_import_empty_markdown_still_returns_a_draft() -> None: + result = import_spec("") + + # Never raises; an empty document just yields an empty-shaped draft. + assert result.manifest is not None + assert result.parse_error is None + + +# --------------------------------------------------------------------------- # +# Endpoint integration tests # +# --------------------------------------------------------------------------- # + + +def _client( + authenticate_app: Callable[..., FastAPI], + *, + role: UserRole = UserRole.ADMIN, + engine: FileSpecEngine | None = None, +) -> TestClient: + app = create_app() + principal = Principal( + user_id=uuid.uuid4(), + workspace_id=uuid.uuid4(), + role=role, + email="test@forge.local", + auth_method="test", + scopes=["*"], + ) + authenticate_app(app, principal) + if engine is not None: + app.dependency_overrides[get_spec_engine] = lambda: engine + return TestClient(app) + + +def test_import_endpoint_returns_normalized_draft( + authenticate_app: Callable[..., FastAPI], +) -> None: + client = _client(authenticate_app) + with client: + resp = client.post( + "/spec/import", + json={ + "content": "# My feature\n\n## Requirements\n- Do the thing\n", + "source_format": "markdown", + }, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["source_format"] == "markdown" + assert body["normalized"] is True + assert body["manifest"]["name"] == "My feature" + assert body["parse_error"] is None + + +def test_import_endpoint_auto_detects_yaml(authenticate_app: Callable[..., FastAPI]) -> None: + client = _client(authenticate_app) + with client: + resp = client.post( + "/spec/import", + json={"content": "title: A yaml spec\nrequirements:\n - Do X\n"}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["source_format"] == "yaml" + assert body["manifest"]["name"] == "A yaml spec" + + +def test_import_endpoint_requires_write_permission( + authenticate_app: Callable[..., FastAPI], +) -> None: + client = _client(authenticate_app, role=UserRole.VIEWER) + with client: + resp = client.post("/spec/import", json={"content": "# Thing\n"}) + assert resp.status_code == 403 + + +def test_import_endpoint_rejects_empty_content( + authenticate_app: Callable[..., FastAPI], +) -> None: + client = _client(authenticate_app) + with client: + resp = client.post("/spec/import", json={"content": ""}) + assert resp.status_code == 422 + + +def test_import_endpoint_does_not_persist( + tmp_path: Path, authenticate_app: Callable[..., FastAPI] +) -> None: + """Draft-only: nothing importable ends up written to the spec engine.""" + engine = FileSpecEngine(root=tmp_path / "specs") + client = _client(authenticate_app, engine=engine) + with client: + resp = client.post( + "/spec/import", + json={"content": "# Thing\n\n## Requirements\n- Do X\n"}, + ) + assert resp.status_code == 200 + spec_uuid = spec_id_for_key(IMPORT_PLACEHOLDER_ID) + fetched = client.get(f"/spec/specs/{spec_uuid}") + assert fetched.status_code == 404 diff --git a/apps/api/tests/test_spec_router.py b/apps/api/tests/test_spec_router.py index 9d2b8b26..0c787bd6 100644 --- a/apps/api/tests/test_spec_router.py +++ b/apps/api/tests/test_spec_router.py @@ -14,18 +14,48 @@ import pytest from fastapi import FastAPI from fastapi.testclient import TestClient +from sqlalchemy import StaticPool, create_engine +from sqlalchemy.orm import Session, sessionmaker +from forge_api.db import get_db from forge_api.main import create_app from forge_api.routers.spec import get_spec_engine +from forge_db.base import Base +from forge_db.models import Workspace from forge_spec import FileSpecEngine, spec_id_for_key +#: Mirrors ``conftest.py``'s deterministic test workspace (tests mirror rather +#: than cross-import conftest constants, per repo convention). +_TEST_WORKSPACE_ID = uuid.UUID("00000000-0000-0000-0000-0000000000a1") + @pytest.fixture def client(tmp_path: Path, authenticate_app: Callable[..., FastAPI]) -> Iterator[TestClient]: app = create_app() authenticate_app(app) engine = FileSpecEngine(root=tmp_path / "specs") + + # ss-versioning: every save also records a ``spec_version`` row, so the + # write endpoints now need a DB session (SQLite in-memory here, mirroring + # ``test_project_spec_overview.py``'s hermetic fixture). + db_engine = create_engine( + "sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool + ) + Base.metadata.create_all(db_engine) + db_factory = sessionmaker(bind=db_engine, expire_on_commit=False, class_=Session) + with db_factory() as session: + session.add(Workspace(id=_TEST_WORKSPACE_ID, name="Acme", slug="acme")) + session.commit() + + def _override_db() -> Iterator[Session]: + session = db_factory() + try: + yield session + finally: + session.close() + app.dependency_overrides[get_spec_engine] = lambda: engine + app.dependency_overrides[get_db] = _override_db with TestClient(app) as c: yield c @@ -70,6 +100,127 @@ def test_tasks_before_approval_is_gated_409(client: TestClient) -> None: assert resp.status_code == 409 +def test_read_missing_constitution_is_404(client: TestClient) -> None: + resp = client.get(f"/spec/constitution/{uuid.uuid4()}") + assert resp.status_code == 404 + + +def test_read_constitution_after_init(client: TestClient) -> None: + project_id = uuid.uuid4() + init = client.post("/spec/constitution", json={"project_id": str(project_id)}) + assert init.status_code == 201, init.text + + resp = client.get(f"/spec/constitution/{project_id}") + + assert resp.status_code == 200, resp.text + assert resp.json()["project_id"] == str(project_id) + assert resp.json()["principles"] == init.json()["principles"] + + +def test_read_spec_markdown_round_trips_the_manifest(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + resp = client.get(f"/spec/specs/{spec_uuid}/markdown") + + assert resp.status_code == 200, resp.text + assert "text/plain" in resp.headers["content-type"] + text = resp.text + assert manifest["id"] in text + assert "Customer search" in text + assert "R1" in text + + +def test_edit_spec_via_markdown_updates_the_manifest(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + text = client.get(f"/spec/specs/{spec_uuid}/markdown").text + + edited = text.replace( + "- **R1**: Search customers by name", + "- **R1**: Search customers by name or email", + ) + resp = client.put(f"/spec/specs/{spec_uuid}/markdown", json={"content": edited}) + + assert resp.status_code == 200, resp.text + updated = resp.json() + assert updated["requirements"][0]["text"] == "Search customers by name or email" + + # manifest.yaml was re-rendered to match. + yaml_text = client.get(f"/spec/specs/{spec_uuid}/manifest").text + assert "Search customers by name or email" in yaml_text + + +def test_read_missing_spec_markdown_is_404(client: TestClient) -> None: + resp = client.get(f"/spec/specs/{uuid.uuid4()}/markdown") + assert resp.status_code == 404 + + +def test_read_spec_manifest_yaml_round_trips_the_manifest(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + resp = client.get(f"/spec/specs/{spec_uuid}/manifest") + + assert resp.status_code == 200, resp.text + assert "text/plain" in resp.headers["content-type"] + assert f"id: {manifest['id']}" in resp.text + assert "Search customers by name" in resp.text + + +def test_create_spec_via_manifest_yaml(client: TestClient) -> None: + """Creating a spec straight from ``manifest.yaml`` (both formats writable).""" + yaml_body = ( + "id: SPEC-99\n" + "name: Billing v2\n" + "status: draft\n" + "requirements:\n" + " - id: R1\n" + " text: Charge a card\n" + ) + spec_uuid = spec_id_for_key("SPEC-99") + + resp = client.put(f"/spec/specs/{spec_uuid}/manifest", json={"content": yaml_body}) + + assert resp.status_code == 200, resp.text + created = resp.json() + assert created["id"] == "SPEC-99" + assert created["name"] == "Billing v2" + + fetched = client.get(f"/spec/specs/{spec_uuid}") + assert fetched.status_code == 200 + assert fetched.json()["name"] == "Billing v2" + + # spec.md was rendered to match the YAML-authored manifest. + md_text = client.get(f"/spec/specs/{spec_uuid}/markdown").text + assert "Billing v2" in md_text + assert "Charge a card" in md_text + + +def test_edit_spec_via_manifest_yaml_updates_the_manifest(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + yaml_text = client.get(f"/spec/specs/{spec_uuid}/manifest").text + + edited = yaml_text.replace( + "text: Search customers by name", "text: Search customers by name, email, or phone" + ) + resp = client.put(f"/spec/specs/{spec_uuid}/manifest", json={"content": edited}) + + assert resp.status_code == 200, resp.text + updated = resp.json() + assert updated["requirements"][0]["text"] == "Search customers by name, email, or phone" + + # spec.md was re-rendered to match. + md_text = client.get(f"/spec/specs/{spec_uuid}/markdown").text + assert "Search customers by name, email, or phone" in md_text + + +def test_read_missing_spec_manifest_yaml_is_404(client: TestClient) -> None: + resp = client.get(f"/spec/specs/{uuid.uuid4()}/manifest") + assert resp.status_code == 404 + + def test_lifecycle_clarify_plan_approve_tasks(client: TestClient) -> None: manifest = _create_spec(client) spec_uuid = spec_id_for_key(manifest["id"]) diff --git a/apps/api/tests/test_spec_versioning.py b/apps/api/tests/test_spec_versioning.py new file mode 100644 index 00000000..987293bd --- /dev/null +++ b/apps/api/tests/test_spec_versioning.py @@ -0,0 +1,214 @@ +"""Integration tests for spec versioning + diff (ss-versioning). + +Every save through the editing endpoints (``spec_create`` / ``write_manifest`` +/ ``write_spec_markdown`` / ``write_spec_manifest_yaml``) appends an immutable +``spec_version`` row; ``GET .../versions`` lists them, ``GET +.../versions/{n}`` reads one snapshot, and ``GET +.../versions/{a}/diff/{b}`` diffs two of them (line-level markdown + +structured manifest). Hermetic: SQLite in-memory backs the DB, a tmp-rooted +``FileSpecEngine`` backs the spec content. +""" + +from __future__ import annotations + +import uuid +from collections.abc import Callable, Iterator +from pathlib import Path + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient +from sqlalchemy import StaticPool, create_engine +from sqlalchemy.orm import Session, sessionmaker + +from forge_api.db import get_db +from forge_api.deps import get_current_principal +from forge_api.main import create_app +from forge_api.routers.spec import get_spec_engine +from forge_db.base import Base +from forge_db.models import Workspace +from forge_spec import FileSpecEngine, spec_id_for_key + +# Deterministic identities mirroring ``conftest.py``'s (tests mirror rather +# than cross-import conftest constants, per repo convention). +TEST_WORKSPACE_ID = uuid.UUID("00000000-0000-0000-0000-0000000000a1") +TEST_USER_ID = uuid.UUID("00000000-0000-0000-0000-0000000000b2") + + +@pytest.fixture +def db_factory() -> Iterator[sessionmaker[Session]]: + engine = create_engine( + "sqlite://", connect_args={"check_same_thread": False}, poolclass=StaticPool + ) + Base.metadata.create_all(engine) + factory = sessionmaker(bind=engine, expire_on_commit=False, class_=Session) + with factory() as session: + session.add(Workspace(id=TEST_WORKSPACE_ID, name="Acme", slug="acme")) + session.commit() + yield factory + + +@pytest.fixture +def client( + tmp_path: Path, + authenticate_app: Callable[..., FastAPI], + db_factory: sessionmaker[Session], +) -> Iterator[TestClient]: + app = create_app() + authenticate_app(app) + engine = FileSpecEngine(root=tmp_path / "specs") + + def _override_db() -> Iterator[Session]: + session = db_factory() + try: + yield session + finally: + session.close() + + app.dependency_overrides[get_spec_engine] = lambda: engine + app.dependency_overrides[get_db] = _override_db + with TestClient(app) as c: + yield c + + +def _create_spec(client: TestClient, name: str = "Customer search") -> dict: + resp = client.post( + "/spec/specs", + json={ + "epic_id": str(uuid.uuid4()), + "name": name, + "requirements": [{"id": "R1", "text": "Search customers by name"}], + }, + ) + assert resp.status_code == 201, resp.text + return resp.json() + + +def test_spec_create_records_version_one(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + resp = client.get(f"/spec/specs/{spec_uuid}/versions") + + assert resp.status_code == 200, resp.text + versions = resp.json() + assert len(versions) == 1 + assert versions[0]["version_number"] == 1 + assert versions[0]["name"] == "Customer search" + assert versions[0]["created_by"] == str(TEST_USER_ID) + + +def test_saving_manifest_appends_a_new_version(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + manifest["name"] = "Customer search v2" + resp = client.put(f"/spec/specs/{spec_uuid}", json=manifest) + assert resp.status_code == 200, resp.text + + versions = client.get(f"/spec/specs/{spec_uuid}/versions").json() + assert [v["version_number"] for v in versions] == [2, 1] + assert versions[0]["name"] == "Customer search v2" + + +def test_saving_markdown_appends_a_new_version(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + markdown = client.get(f"/spec/specs/{spec_uuid}/markdown").text + updated_markdown = markdown.replace( + "Search customers by name", "Search customers by name or email" + ) + resp = client.put( + f"/spec/specs/{spec_uuid}/markdown", + json={"content": updated_markdown}, + ) + assert resp.status_code == 200, resp.text + + versions = client.get(f"/spec/specs/{spec_uuid}/versions").json() + assert len(versions) == 2 + + +def test_read_one_version_returns_full_snapshot(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + resp = client.get(f"/spec/specs/{spec_uuid}/versions/1") + + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["version_number"] == 1 + assert body["manifest"]["name"] == "Customer search" + assert "Customer search" in body["spec_md"] + assert "Customer search" in body["manifest_yaml"] + + +def test_read_missing_version_is_404(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + resp = client.get(f"/spec/specs/{spec_uuid}/versions/99") + + assert resp.status_code == 404 + + +def test_diff_two_versions_reports_markdown_and_manifest_changes(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + manifest["name"] = "Customer search v2" + manifest["requirements"].append({"id": "R2", "text": "Filter by status"}) + client.put(f"/spec/specs/{spec_uuid}", json=manifest) + + resp = client.get(f"/spec/specs/{spec_uuid}/versions/1/diff/2") + + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["from_version"] == 1 + assert body["to_version"] == 2 + assert any(line["op"] == "insert" for line in body["markdown"]) + assert any(line["op"] == "delete" for line in body["markdown"]) + scalar_fields = {c["field"] for c in body["manifest"]["scalar_changes"]} + assert "name" in scalar_fields + added_requirement_ids = { + c["id"] for c in body["manifest"]["requirements"] if c["change"] == "added" + } + assert "R2" in added_requirement_ids + + +def test_diff_missing_version_is_404(client: TestClient) -> None: + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + resp = client.get(f"/spec/specs/{spec_uuid}/versions/1/diff/2") + + assert resp.status_code == 404 + + +def test_versions_are_workspace_scoped( + client: TestClient, db_factory: sessionmaker[Session] +) -> None: + other_workspace = uuid.uuid4() + with db_factory() as session: + session.add(Workspace(id=other_workspace, name="Other", slug="other")) + session.commit() + + manifest = _create_spec(client) + spec_uuid = spec_id_for_key(manifest["id"]) + + # A different workspace never sees this spec's versions (empty, not 404 — + # mirrors the F23 dashboard's "no linked specs" convention). + from forge_api.deps import Principal + from forge_contracts import UserRole + + other_principal = Principal( + user_id=uuid.uuid4(), + workspace_id=other_workspace, + role=UserRole.ADMIN, + auth_method="test", + scopes=["*"], + ) + client.app.dependency_overrides[get_current_principal] = lambda: other_principal + resp = client.get(f"/spec/specs/{spec_uuid}/versions") + assert resp.status_code == 200 + assert resp.json() == [] diff --git a/apps/web/package.json b/apps/web/package.json index 3bf7a081..9dd86e9b 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -28,7 +28,8 @@ "next": "^16.2.9", "react": "^19.2.7", "react-dom": "^19.2.7", - "tailwind-merge": "^3.6.0" + "tailwind-merge": "^3.6.0", + "yaml": "^2.9.0" }, "devDependencies": { "@tailwindcss/postcss": "^4.3.1", diff --git a/apps/web/src/app/(board)/specs/[id]/page.tsx b/apps/web/src/app/(board)/specs/[id]/page.tsx new file mode 100644 index 00000000..f70133dc --- /dev/null +++ b/apps/web/src/app/(board)/specs/[id]/page.tsx @@ -0,0 +1,14 @@ +import { SpecStudioPage } from "@/components/spec-studio/spec-studio-page"; + +/** + * `/specs/{id}` — a dedicated, deep-linkable Spec Studio for one spec, + * defaulting to Guided mode. + */ +export default async function SpecRoute({ + params, +}: { + params: Promise<{ id: string }>; +}) { + const { id } = await params; + return ; +} diff --git a/apps/web/src/app/(board)/specs/new/page.tsx b/apps/web/src/app/(board)/specs/new/page.tsx new file mode 100644 index 00000000..d1495dd9 --- /dev/null +++ b/apps/web/src/app/(board)/specs/new/page.tsx @@ -0,0 +1,19 @@ +import { Suspense } from "react"; + +import { NewSpecPage } from "@/components/spec-studio/new-spec-page"; + +/** + * `/specs/new` — the guided spec-creation entry point (pick an epic, draft + * the goal/requirements/acceptance criteria via the Guided-mode form). + * + * Wrapped in `Suspense`: `NewSpecPage` reads `?epicId=` via `useSearchParams` + * (the board-epic "Create spec" entry point preselects the epic), which Next + * requires a Suspense boundary around for static export. + */ +export default function NewSpecRoute() { + return ( + + + + ); +} diff --git a/apps/web/src/components/board/depth/depth-roadmap.test.tsx b/apps/web/src/components/board/depth/depth-roadmap.test.tsx index 91f8b6e5..45e1c1a0 100644 --- a/apps/web/src/components/board/depth/depth-roadmap.test.tsx +++ b/apps/web/src/components/board/depth/depth-roadmap.test.tsx @@ -31,4 +31,20 @@ describe("DepthRoadmap", () => { expect(screen.getByTestId("milestone-m1")).toHaveTextContent("Beta"); expect(within(screen.getByTestId("cell-e1-s1")).getByText("Login")).toBeInTheDocument(); }); + + it("gives every real epic lane a 'Create spec' action pointing at /specs/new", () => { + render( + , + ); + const link = screen.getByTestId("lane-create-spec-e1"); + expect(link).toHaveAttribute("href", "/specs/new?epicId=e1"); + }); + + it("does not offer 'Create spec' on the synthetic 'No epic' lane", () => { + const unepiced: TaskDTO[] = [{ id: "t2", title: "Stray", status: "backlog" }]; + render( + , + ); + expect(screen.queryByTestId(/lane-create-spec-__no_epic__/)).not.toBeInTheDocument(); + }); }); diff --git a/apps/web/src/components/board/depth/depth-roadmap.tsx b/apps/web/src/components/board/depth/depth-roadmap.tsx index abf90b13..15769333 100644 --- a/apps/web/src/components/board/depth/depth-roadmap.tsx +++ b/apps/web/src/components/board/depth/depth-roadmap.tsx @@ -1,6 +1,7 @@ "use client"; -import { Flag } from "lucide-react"; +import { FilePlus2, Flag } from "lucide-react"; +import Link from "next/link"; import type { EpicDTO, @@ -10,7 +11,7 @@ import type { TaskStatus, } from "@/lib/api/types"; import { STATUS_LABELS } from "@/lib/board/status"; -import { buildRoadmap, type RoadmapColumn } from "@/lib/board/roadmap"; +import { buildRoadmap, NO_EPIC_ID, type RoadmapColumn } from "@/lib/board/roadmap"; import { cn } from "@/lib/utils"; export interface DepthRoadmapProps { @@ -167,13 +168,25 @@ interface RoadmapLaneRowProps { } function RoadmapLaneRow({ laneId, label, columns, cells }: RoadmapLaneRowProps) { + const isRealEpic = laneId !== NO_EPIC_ID; return ( <>
{label} + {isRealEpic ? ( + + + + ) : null}
{columns.map((column) => { const cellTasks = cells[column.id] ?? []; diff --git a/apps/web/src/components/spec-studio/ai-draft-panel.test.tsx b/apps/web/src/components/spec-studio/ai-draft-panel.test.tsx new file mode 100644 index 00000000..af07f77c --- /dev/null +++ b/apps/web/src/components/spec-studio/ai-draft-panel.test.tsx @@ -0,0 +1,153 @@ +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import type { ReactNode } from "react"; +import { describe, expect, it, vi } from "vitest"; + +import type { ForgeApiClient } from "@/lib/api/client"; +import type { SpecDraft } from "@/lib/api/types"; + +import { AiDraftPanel } from "./ai-draft-panel"; + +const draft: SpecDraft = { + goal: "Let customers search orders by name", + model: "claude-opus-4-8", + spec_md: "---\nid: SPEC-DRAFT\nstatus: draft\n---\n\n## Goal\n\nSearch orders by name\n", + manifest: { + id: "SPEC-DRAFT", + name: "Search orders by name", + status: "draft", + requirements: [{ id: "R1", text: "Search orders by customer name" }], + }, + usage: { input_tokens: 120, output_tokens: 340, cost_usd: 0.0123, calls: 1 }, +}; + +function makeClient(overrides: Partial = {}): ForgeApiClient { + return { + draftSpec: vi.fn(() => Promise.resolve(draft)), + ...overrides, + } as unknown as ForgeApiClient; +} + +// Real timers throughout (fake timers + RTL's `waitFor` polling deadlock one +// another): a 1ms reveal interval with a tiny chunk size still exercises the +// progressive-reveal behaviour (asserting an early, partial frame) while +// keeping the test fast and using real setTimeout/setInterval end to end. +function renderPanel(client: ForgeApiClient, onDraft = vi.fn()) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + function Wrapper({ children }: { children: ReactNode }) { + return {children}; + } + return { + onDraft, + ...render( + , + { wrapper: Wrapper }, + ), + }; +} + +describe("AiDraftPanel", () => { + it("disables the draft button until a goal is typed", () => { + const client = makeClient(); + renderPanel(client); + expect(screen.getByTestId("ai-draft-submit")).toBeDisabled(); + + fireEvent.change(screen.getByTestId("ai-draft-goal"), { + target: { value: "Search orders by name" }, + }); + expect(screen.getByTestId("ai-draft-submit")).toBeEnabled(); + }); + + it("drafts, streams the spec.md into view, and hands off the parsed manifest once the reveal settles", async () => { + const client = makeClient(); + const { onDraft } = renderPanel(client); + + fireEvent.change(screen.getByTestId("ai-draft-goal"), { + target: { value: "Search orders by name" }, + }); + fireEvent.click(screen.getByTestId("ai-draft-submit")); + + await waitFor(() => + expect(client.draftSpec).toHaveBeenCalledWith( + expect.objectContaining({ goal: "Search orders by name" }), + ), + ); + + // Settles on the full drafted text (revealed progressively via a reveal + // interval, exercised at the unit level by `revealChunkSize`/ + // `revealIntervalMs` — see the component doc), then hands the completed + // draft off exactly once, never before the stream has caught up. + await waitFor(() => + expect(screen.getByTestId("ai-draft-stream").textContent).toBe(draft.spec_md), + ); + await waitFor(() => expect(onDraft).toHaveBeenCalledTimes(1)); + expect(onDraft).toHaveBeenCalledWith(draft); + }); + + it("surfaces the resolved model/tier and estimated cost", async () => { + const client = makeClient(); + renderPanel(client); + + fireEvent.change(screen.getByTestId("ai-draft-goal"), { + target: { value: "Search orders by name" }, + }); + fireEvent.click(screen.getByTestId("ai-draft-submit")); + + await waitFor(() => expect(screen.getByTestId("ai-draft-model")).toBeInTheDocument()); + expect(screen.getByTestId("ai-draft-model").textContent).toContain("claude-opus-4-8"); + expect(screen.getByTestId("ai-draft-model").textContent).toContain("senior tier"); + expect(screen.getByTestId("ai-draft-cost").textContent).toContain("0.0123"); + }); + + it("marks the result as a draft to refine, never auto-saved", async () => { + const client = makeClient(); + renderPanel(client); + + fireEvent.change(screen.getByTestId("ai-draft-goal"), { + target: { value: "Search orders by name" }, + }); + fireEvent.click(screen.getByTestId("ai-draft-submit")); + + await waitFor(() => + expect(screen.getByTestId("ai-draft-badge").textContent).toContain("review before saving"), + ); + }); + + it("surfaces a parse error without losing the raw draft text", async () => { + const badDraft: SpecDraft = { + ...draft, + manifest: null, + parse_error: "missing frontmatter", + }; + const client = makeClient({ draftSpec: vi.fn(() => Promise.resolve(badDraft)) }); + const { onDraft } = renderPanel(client); + + fireEvent.change(screen.getByTestId("ai-draft-goal"), { + target: { value: "Search orders by name" }, + }); + fireEvent.click(screen.getByTestId("ai-draft-submit")); + + await waitFor(() => expect(screen.getByTestId("ai-draft-parse-error")).toBeInTheDocument()); + await waitFor(() => expect(onDraft).toHaveBeenCalledWith(badDraft)); + }); + + it("surfaces a request error", async () => { + const client = makeClient({ + draftSpec: vi.fn(() => Promise.reject(new Error("no model provider configured"))), + }); + renderPanel(client); + + fireEvent.change(screen.getByTestId("ai-draft-goal"), { + target: { value: "Search orders by name" }, + }); + fireEvent.click(screen.getByTestId("ai-draft-submit")); + + await waitFor(() => + expect(screen.getByTestId("ai-draft-error").textContent).toContain( + "no model provider configured", + ), + ); + }); +}); diff --git a/apps/web/src/components/spec-studio/ai-draft-panel.tsx b/apps/web/src/components/spec-studio/ai-draft-panel.tsx new file mode 100644 index 00000000..b44b2f0d --- /dev/null +++ b/apps/web/src/components/spec-studio/ai-draft-panel.tsx @@ -0,0 +1,189 @@ +"use client"; + +import { Sparkles } from "lucide-react"; +import { useEffect, useRef, useState } from "react"; + +import { Button } from "@/components/ui/button"; +import { ApiError, apiClient, type ForgeApiClient } from "@/lib/api/client"; +import { useDraftSpec } from "@/lib/api/spec-studio"; +import type { SpecDraft } from "@/lib/api/types"; + +export interface AiDraftPanelProps { + epicId?: string; + projectId?: string; + client?: ForgeApiClient; + /** + * Called once the drafted `spec_md` has fully streamed in, handing the + * caller the raw prose plus its parsed `SpecManifest` preview so it can + * seed the Guided or Markdown editor. + */ + onDraft: (draft: SpecDraft) => void; + /** ms between reveal ticks (test hook; production default reads as "typing"). */ + revealIntervalMs?: number; + /** Characters revealed per tick (test hook). */ + revealChunkSize?: number; +} + +function errorMessage(error: unknown): string { + if (error instanceof ApiError) return error.message; + if (error instanceof Error) return error.message; + return "Something went wrong"; +} + +/** + * `ss-ai-panel` — the AI draft-from-a-sentence entry point. Type a one-line + * goal; `POST /spec/draft` asks the workspace's BYOK model (routed by the + * Adaptive Orchestration model router, seeded with the project constitution) + * to write a `spec.md`. The full draft comes back in one response (the + * provider-side streaming already happened inside the backend call), but it + * is *revealed* here character-by-character so the authoring experience reads + * as the model "typing" the draft live rather than a page reflow. + * + * The result is always clearly marked as a draft to refine — nothing is + * auto-saved. Once the reveal settles, `onDraft` hands the caller the parsed + * manifest preview + raw `spec_md` so it can populate the Guided/Markdown + * editor. The resolved model (provider + the fixed senior authoring tier) and + * the estimated cost of the call are surfaced alongside the draft. + */ +export function AiDraftPanel({ + epicId, + projectId, + client = apiClient, + onDraft, + revealIntervalMs = 20, + revealChunkSize = 12, +}: AiDraftPanelProps) { + const [goal, setGoal] = useState(""); + const [revealed, setRevealed] = useState(""); + const draftSpec = useDraftSpec(client); + const timerRef = useRef | null>(null); + + const fullText = draftSpec.data?.spec_md ?? ""; + const streaming = draftSpec.isSuccess && revealed.length < fullText.length; + + useEffect( + () => () => { + if (timerRef.current) clearInterval(timerRef.current); + }, + [], + ); + + function handleDraft() { + const trimmed = goal.trim(); + if (!trimmed) return; + setRevealed(""); + if (timerRef.current) { + clearInterval(timerRef.current); + timerRef.current = null; + } + draftSpec.mutate( + { goal: trimmed, epic_id: epicId, project_id: projectId }, + { + onSuccess: (result) => { + const text = result.spec_md ?? ""; + if (!text) return; + let index = 0; + timerRef.current = setInterval(() => { + index = Math.min(text.length, index + revealChunkSize); + setRevealed(text.slice(0, index)); + // Hand off to the caller only once the live reveal has fully + // caught up with the drafted text (not the instant the response + // arrived), so the caller only ever sees the "completed" draft — + // matching what the user just watched stream in — and exactly + // once per draft. + if (index >= text.length) { + if (timerRef.current) { + clearInterval(timerRef.current); + timerRef.current = null; + } + onDraft(result); + } + }, revealIntervalMs); + }, + }, + ); + } + + const usage = draftSpec.data?.usage; + + return ( +
+
+ +

Draft with AI

+
+

+ Describe the goal in one line — a draft spec.md streams in below. It’s a starting + point: review and refine before saving. +

+
+ setGoal(event.target.value)} + placeholder="e.g. Let customers search orders by name" + disabled={draftSpec.isPending} + className="flex-1 rounded-md border border-border bg-card px-3 py-2 text-sm text-foreground outline-none focus-visible:ring-2 focus-visible:ring-ring" + /> + +
+ + {draftSpec.isError ? ( +

+ {errorMessage(draftSpec.error)} +

+ ) : null} + + {draftSpec.isSuccess ? ( +
+
+ + Draft — review before saving + + + {draftSpec.data.model} · senior tier + + {typeof usage?.cost_usd === "number" ? ( + + ${usage.cost_usd.toFixed(4)} + + ) : null} + {streaming ? Streaming… : null} +
+ 
+            {revealed}
+
+ {draftSpec.data.parse_error ? ( +

+ Draft didn’t fully parse: {draftSpec.data.parse_error}. You can still edit it + as Markdown. +

+ ) : null} +
+ ) : null} +
+ ); +} diff --git a/apps/web/src/components/spec-studio/guided-helpers.test.ts b/apps/web/src/components/spec-studio/guided-helpers.test.ts new file mode 100644 index 00000000..2d2b7954 --- /dev/null +++ b/apps/web/src/components/spec-studio/guided-helpers.test.ts @@ -0,0 +1,197 @@ +import { describe, expect, it } from "vitest"; + +import type { SpecManifest } from "@/lib/api/types"; + +import { + addAcceptanceCriterion, + addAdr, + addRequirement, + classifyCriterionStyle, + composeChecklist, + composeGivenWhenThen, + computeChecklist, + computeCoverage, + computeNudges, + convertCriterionText, + nextSequentialId, + parseChecklist, + parseGivenWhenThen, +} from "./guided-helpers"; + +describe("nextSequentialId", () => { + it("returns R1 for an empty list", () => { + expect(nextSequentialId("R", [])).toBe("R1"); + }); + + it("returns one past the highest existing number", () => { + expect(nextSequentialId("R", ["R1", "R2", "R5"])).toBe("R6"); + }); + + it("ignores ids that don't match the prefix pattern", () => { + expect(nextSequentialId("AC", ["R1", "AC3", "custom-id"])).toBe("AC4"); + }); +}); + +describe("Given/When/Then round trip", () => { + it("composes then parses back to the same parts", () => { + const parts = { given: "a user", when: "they sign in", then: "they land on the board" }; + const text = composeGivenWhenThen(parts); + expect(text).toBe("Given a user When they sign in Then they land on the board"); + expect(parseGivenWhenThen(text)).toEqual(parts); + }); + + it("treats unstructured text as the Then clause", () => { + expect(parseGivenWhenThen("just some prose")).toEqual({ + given: "", + when: "", + then: "just some prose", + }); + }); + + it("omits empty parts when composing", () => { + expect(composeGivenWhenThen({ given: "", when: "", then: "it works" })).toBe("Then it works"); + }); +}); + +describe("classifyCriterionStyle", () => { + it("defaults blank text to gherkin (the editor's default shape)", () => { + expect(classifyCriterionStyle("")).toBe("gherkin"); + expect(classifyCriterionStyle(" ")).toBe("gherkin"); + }); + + it("classifies Given/When/Then prose as gherkin", () => { + expect(classifyCriterionStyle("Given a user When they sign in Then the board loads")).toBe("gherkin"); + expect(classifyCriterionStyle("Then it works")).toBe("gherkin"); + }); + + it("classifies a keyword-free sentence as a plain assertion", () => { + expect(classifyCriterionStyle("The endpoint returns 200 for a valid token")).toBe("assertion"); + }); + + it("classifies check-item lines as a checklist, even when a label says 'when'", () => { + expect(classifyCriterionStyle("- [ ] Logs an event when it runs\n- [x] Retries on failure")).toBe( + "checklist", + ); + }); +}); + +describe("checklist (de)serialisation", () => { + it("composes then parses back to the same items", () => { + const items = [ + { label: "Email field validates", checked: false }, + { label: "Password is masked", checked: true }, + ]; + const text = composeChecklist(items); + expect(text).toBe("- [ ] Email field validates\n- [x] Password is masked"); + expect(parseChecklist(text)).toEqual(items); + }); + + it("renders an empty label without a trailing space", () => { + expect(composeChecklist([{ label: "", checked: false }])).toBe("- [ ]"); + }); +}); + +describe("convertCriterionText", () => { + it("wraps prose into a single unchecked item when switching to a checklist", () => { + expect(convertCriterionText("Then it works", "checklist")).toBe("- [ ] Then it works"); + }); + + it("joins checklist labels back into prose when leaving the checklist style", () => { + const text = "- [ ] first\n- [x] second"; + expect(convertCriterionText(text, "assertion")).toBe("first; second"); + expect(convertCriterionText(text, "gherkin")).toBe("first second"); + }); + + it("is a no-op between gherkin and assertion (shared flat prose)", () => { + expect(convertCriterionText("The system does X", "gherkin")).toBe("The system does X"); + }); +}); + +describe("computeNudges", () => { + const base: SpecManifest = { id: "s1", name: "Passwordless auth" }; + + it("nudges an empty goal and missing requirements", () => { + const nudges = computeNudges({ ...base, name: "" }); + expect(nudges.map((n) => n.id)).toEqual(expect.arrayContaining(["no-goal", "no-requirements"])); + }); + + it("nudges a requirement with no linked acceptance criterion", () => { + const nudges = computeNudges({ + ...base, + requirements: [{ id: "R1", text: "Sign in" }], + acceptance_criteria: [], + }); + expect(nudges.some((n) => n.id === "uncovered-R1")).toBe(true); + }); + + it("has no coverage nudges once every requirement is linked", () => { + const nudges = computeNudges({ + ...base, + requirements: [{ id: "R1", text: "Sign in" }], + acceptance_criteria: [{ id: "AC1", text: "Given...", req_refs: ["R1"] }], + }); + expect(nudges.some((n) => n.id.startsWith("uncovered-"))).toBe(false); + expect(nudges.some((n) => n.id === "unlinked-AC1")).toBe(false); + }); + + it("nudges an unresolved open question", () => { + const nudges = computeNudges({ + ...base, + open_questions: [{ id: "Q1", text: "Which provider?" }], + }); + expect(nudges.some((n) => n.id === "open-questions")).toBe(true); + }); +}); + +describe("computeCoverage", () => { + it("is 0/0 with no requirements", () => { + expect(computeCoverage({ id: "s1", name: "x" })).toEqual({ satisfied: 0, total: 0, pct: 0 }); + }); + + it("computes the satisfied fraction", () => { + const coverage = computeCoverage({ + id: "s1", + name: "x", + requirements: [ + { id: "R1", text: "a" }, + { id: "R2", text: "b" }, + ], + acceptance_criteria: [{ id: "AC1", text: "t", req_refs: ["R1"] }], + }); + expect(coverage).toEqual({ satisfied: 1, total: 2, pct: 50 }); + }); +}); + +describe("computeChecklist", () => { + it("is all incomplete for an empty manifest", () => { + const items = computeChecklist({ id: "s1", name: "" }); + expect(items.every((i) => !i.done)).toBe(true); + }); + + it("is all complete for a fully covered manifest", () => { + const items = computeChecklist({ + id: "s1", + name: "Passwordless auth", + requirements: [{ id: "R1", text: "a" }], + acceptance_criteria: [{ id: "AC1", text: "t", req_refs: ["R1"] }], + }); + expect(items.every((i) => i.done)).toBe(true); + }); +}); + +describe("add* helpers", () => { + it("addRequirement appends the next sequential requirement", () => { + expect(addRequirement([{ id: "R1", text: "a" }])).toEqual([ + { id: "R1", text: "a" }, + { id: "R2", text: "" }, + ]); + }); + + it("addAcceptanceCriterion appends the next sequential AC with empty req_refs", () => { + expect(addAcceptanceCriterion([])).toEqual([{ id: "AC1", text: "", req_refs: [] }]); + }); + + it("addAdr appends the next sequential ADR", () => { + expect(addAdr([])).toEqual([{ id: "ADR1", title: "", status: "proposed" }]); + }); +}); diff --git a/apps/web/src/components/spec-studio/guided-helpers.ts b/apps/web/src/components/spec-studio/guided-helpers.ts new file mode 100644 index 00000000..9e109414 --- /dev/null +++ b/apps/web/src/components/spec-studio/guided-helpers.ts @@ -0,0 +1,254 @@ +/** + * Pure helpers backing Guided mode — auto-numbering, Given/When/Then + * composition, and the soft validation nudges + Ready-to-create checklist / + * coverage meter. Kept dependency-free (no React) so they're trivially unit + * tested and reusable from both the form and its summary panel. + */ + +import type { AcceptanceCriterion, ADR, Requirement, SpecManifest } from "@/lib/api/types"; + +/** + * The next sequential id for a prefix (`"R"` / `"AC"` / `"ADR"`) given the ids + * already in use — scans for `${prefix}` and returns one past the + * highest match (or `${prefix}1` when none match), so ids stay auto-numbered + * even after items in the middle are removed. + */ +export function nextSequentialId(prefix: string, existingIds: readonly string[]): string { + const pattern = new RegExp(`^${prefix}(\\d+)$`); + let max = 0; + for (const id of existingIds) { + const match = pattern.exec(id); + if (match) { + const n = Number.parseInt(match[1], 10); + if (n > max) max = n; + } + } + return `${prefix}${max + 1}`; +} + +export interface GivenWhenThen { + given: string; + when: string; + then: string; +} + +/** + * Best-effort split of an AC's free-text `text` into Given/When/Then parts. + * Each keyword is matched independently (not as one all-or-nothing pattern), + * so a still-partial edit — e.g. only "Given ..." typed so far — round-trips + * without losing what's already there. + */ +export function parseGivenWhenThen(text: string): GivenWhenThen { + const trimmed = text.trim(); + const givenMatch = /Given\s+(.*?)(?=\s+When\s+|\s+Then\s+|$)/is.exec(trimmed); + const whenMatch = /When\s+(.*?)(?=\s+Then\s+|$)/is.exec(trimmed); + const thenMatch = /Then\s+(.*)$/is.exec(trimmed); + if (!givenMatch && !whenMatch && !thenMatch) { + return { given: "", when: "", then: trimmed }; + } + return { + given: givenMatch ? givenMatch[1].trim() : "", + when: whenMatch ? whenMatch[1].trim() : "", + then: thenMatch ? thenMatch[1].trim() : "", + }; +} + +/** Compose Given/When/Then parts back into the AC's single `text` field. */ +export function composeGivenWhenThen({ given, when, then }: GivenWhenThen): string { + const parts: string[] = []; + if (given) parts.push(`Given ${given}`); + if (when) parts.push(`When ${when}`); + if (then) parts.push(`Then ${then}`); + return parts.join(" "); +} + +/** + * The three first-class acceptance-criterion authoring styles. Every style is + * encoded losslessly inside the criterion's single `text` field, so switching + * style never touches its `req_refs` (R#) links. Mirrors + * `forge_spec.criteria.classify_criterion` on the backend. + */ +export type CriterionStyle = "gherkin" | "assertion" | "checklist"; + +/** `- [ ] label` / `- [x] label` — the checked box is case-insensitive. */ +const CHECK_ITEM = /^- \[([ xX])\] ?(.*)$/; +const GHERKIN_KEYWORD = /\b(?:given|when|then)\b/i; + +/** One checklist entry: a `label` and whether its box is `checked`. */ +export interface CheckItem { + label: string; + checked: boolean; +} + +function nonBlankLines(text: string): string[] { + return text + .split("\n") + .map((line) => line.trim()) + .filter((line) => line.length > 0); +} + +/** + * Infer a criterion's authoring style from its `text` (never throws). Blank + * text defaults to `"gherkin"` (the editor's default shape); a `text` whose + * every non-blank line is a check item is a `"checklist"` even if a label + * contains a Gherkin keyword; otherwise Gherkin keywords => `"gherkin"`, and + * anything else is a plain `"assertion"`. + */ +export function classifyCriterionStyle(text: string): CriterionStyle { + const lines = nonBlankLines(text); + if (lines.length === 0) return "gherkin"; + if (lines.every((line) => CHECK_ITEM.test(line))) return "checklist"; + if (GHERKIN_KEYWORD.test(text)) return "gherkin"; + return "assertion"; +} + +/** Parse checklist `text` into items (a non-item line becomes an unchecked item). */ +export function parseChecklist(text: string): CheckItem[] { + return nonBlankLines(text).map((line) => { + const match = CHECK_ITEM.exec(line); + if (!match) return { label: line, checked: false }; + return { label: match[2].trim(), checked: match[1] === "x" || match[1] === "X" }; + }); +} + +/** Render checklist `items` back to canonical `- [ ] label` lines. */ +export function composeChecklist(items: readonly CheckItem[]): string { + return items.map((item) => `- [${item.checked ? "x" : " "}] ${item.label}`.trimEnd()).join("\n"); +} + +/** + * Re-encode a criterion's `text` for a new `target` style, preserving prose + * where it makes sense so switching styles doesn't silently lose content. + */ +export function convertCriterionText(text: string, target: CriterionStyle): string { + const current = classifyCriterionStyle(text); + if (current === target) return text; + if (target === "checklist") { + const label = text.trim(); + return composeChecklist([{ label, checked: false }]); + } + if (current === "checklist") { + const labels = parseChecklist(text) + .map((item) => item.label) + .filter(Boolean); + // Gherkin editor will re-parse the joined prose; assertion keeps it flat. + return labels.join(target === "gherkin" ? " " : "; "); + } + // gherkin <-> assertion share the same flat prose encoding. + return text; +} + +/** A single soft-validation nudge — never blocking, just surfaced guidance. */ +export interface Nudge { + id: string; + message: string; +} + +/** Non-blocking nudges: gaps a human should notice before creating the spec. */ +export function computeNudges(manifest: SpecManifest): Nudge[] { + const nudges: Nudge[] = []; + const requirements = manifest.requirements ?? []; + const criteria = manifest.acceptance_criteria ?? []; + + if (!manifest.name.trim()) { + nudges.push({ id: "no-goal", message: "The goal is empty — describe what this spec achieves." }); + } + if (requirements.length === 0) { + nudges.push({ id: "no-requirements", message: "Add at least one requirement." }); + } + if (requirements.length > 0 && criteria.length === 0) { + nudges.push({ + id: "no-criteria", + message: "Add acceptance criteria so requirements can be verified.", + }); + } + for (const req of requirements) { + const linked = criteria.some((ac) => (ac.req_refs ?? []).includes(req.id)); + if (!linked) { + nudges.push({ + id: `uncovered-${req.id}`, + message: `${req.id} has no linked acceptance criterion.`, + }); + } + } + for (const req of requirements) { + if (!req.text.trim()) { + nudges.push({ id: `empty-req-${req.id}`, message: `${req.id} has no description yet.` }); + } + } + for (const ac of criteria) { + if ((ac.req_refs ?? []).length === 0) { + nudges.push({ id: `unlinked-${ac.id}`, message: `${ac.id} isn't linked to a requirement.` }); + } + } + const openQuestions = manifest.open_questions ?? []; + const unresolved = openQuestions.filter((q) => !q.resolution); + if (unresolved.length > 0) { + nudges.push({ + id: "open-questions", + message: `${unresolved.length} open question${unresolved.length === 1 ? "" : "s"} still unresolved.`, + }); + } + return nudges; +} + +/** Requirement coverage: the fraction of requirements with >=1 linked AC. */ +export interface CoverageSummary { + satisfied: number; + total: number; + pct: number; +} + +export function computeCoverage(manifest: SpecManifest): CoverageSummary { + const requirements = manifest.requirements ?? []; + const criteria = manifest.acceptance_criteria ?? []; + const total = requirements.length; + const satisfied = requirements.filter((req) => + criteria.some((ac) => (ac.req_refs ?? []).includes(req.id)), + ).length; + const pct = total > 0 ? Math.round((satisfied / total) * 100) : 0; + return { satisfied, total, pct }; +} + +/** One line of the Ready-to-create checklist. */ +export interface ChecklistItem { + id: string; + label: string; + done: boolean; +} + +/** The Ready-to-create checklist — the minimum bar for a spec worth reviewing. */ +export function computeChecklist(manifest: SpecManifest): ChecklistItem[] { + const requirements = manifest.requirements ?? []; + const criteria = manifest.acceptance_criteria ?? []; + const coverage = computeCoverage(manifest); + return [ + { id: "goal", label: "Goal is filled in", done: manifest.name.trim().length > 0 }, + { id: "requirements", label: "At least one requirement", done: requirements.length > 0 }, + { + id: "criteria", + label: "At least one acceptance criterion", + done: criteria.length > 0, + }, + { + id: "coverage", + label: "Every requirement has a linked acceptance criterion", + done: requirements.length > 0 && coverage.satisfied === coverage.total, + }, + ]; +} + +export function addRequirement(requirements: Requirement[]): Requirement[] { + const id = nextSequentialId("R", requirements.map((r) => r.id)); + return [...requirements, { id, text: "" }]; +} + +export function addAcceptanceCriterion(criteria: AcceptanceCriterion[]): AcceptanceCriterion[] { + const id = nextSequentialId("AC", criteria.map((c) => c.id)); + return [...criteria, { id, text: "", req_refs: [] }]; +} + +export function addAdr(decisions: ADR[]): ADR[] { + const id = nextSequentialId("ADR", decisions.map((d) => d.id)); + return [...decisions, { id, title: "", status: "proposed" }]; +} diff --git a/apps/web/src/components/spec-studio/guided-mode.test.tsx b/apps/web/src/components/spec-studio/guided-mode.test.tsx new file mode 100644 index 00000000..b9498b1f --- /dev/null +++ b/apps/web/src/components/spec-studio/guided-mode.test.tsx @@ -0,0 +1,191 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { useState } from "react"; +import { describe, expect, it, vi } from "vitest"; + +import type { SpecManifest } from "@/lib/api/types"; + +import { GuidedMode } from "./guided-mode"; + +function Harness({ initial }: { initial: SpecManifest }) { + const [value, setValue] = useState(initial); + return ; +} + +const baseManifest: SpecManifest = { + id: "s1", + name: "Passwordless auth", + requirements: [{ id: "R1", text: "Sign in without a password" }], +}; + +describe("GuidedMode", () => { + it("renders the Goal, Requirements, Acceptance Criteria and Constraints blocks", () => { + render(); + expect(screen.getByTestId("guided-name")).toHaveValue("Passwordless auth"); + expect(screen.getByTestId("guided-requirements")).toBeInTheDocument(); + expect(screen.getByTestId("guided-acceptance-criteria")).toBeInTheDocument(); + expect(screen.getByTestId("guided-constraints")).toBeInTheDocument(); + }); + + it("auto-numbers a newly added requirement without a text input for its id", () => { + render(); + fireEvent.click(screen.getByTestId("guided-add-requirement")); + expect(screen.getByTestId("requirement-id-0")).toHaveTextContent("R1"); + fireEvent.click(screen.getByTestId("guided-add-requirement")); + expect(screen.getByTestId("requirement-id-1")).toHaveTextContent("R2"); + }); + + it("adds an acceptance criterion with Given/When/Then fields and auto-numbered id", () => { + render(); + fireEvent.click(screen.getByTestId("guided-add-acceptance-criterion")); + expect(screen.getByTestId("acceptance-criterion-id-0")).toHaveTextContent("AC1"); + fireEvent.change(screen.getByLabelText("AC1 given"), { target: { value: "a user" } }); + fireEvent.change(screen.getByLabelText("AC1 when"), { target: { value: "they sign in" } }); + fireEvent.change(screen.getByLabelText("AC1 then"), { target: { value: "they land on the board" } }); + expect(screen.getByLabelText("AC1 given")).toHaveValue("a user"); + expect(screen.getByLabelText("AC1 when")).toHaveValue("they sign in"); + expect(screen.getByLabelText("AC1 then")).toHaveValue("they land on the board"); + }); + + it("defaults a new acceptance criterion to the Given/When/Then style", () => { + render(); + fireEvent.click(screen.getByTestId("guided-add-acceptance-criterion")); + expect(screen.getByTestId("ac-style-0")).toHaveValue("gherkin"); + expect(screen.getByLabelText("AC1 given")).toBeInTheDocument(); + }); + + it("switches an acceptance criterion to the plain-assertion style", () => { + render(); + fireEvent.click(screen.getByTestId("guided-add-acceptance-criterion")); + fireEvent.change(screen.getByTestId("ac-style-0"), { target: { value: "assertion" } }); + expect(screen.queryByLabelText("AC1 given")).not.toBeInTheDocument(); + const input = screen.getByLabelText("AC1 assertion"); + fireEvent.change(input, { target: { value: "The endpoint returns 200" } }); + expect(input).toHaveValue("The endpoint returns 200"); + }); + + it("switches to the checklist style and edits check items", () => { + render(); + fireEvent.click(screen.getByTestId("guided-add-acceptance-criterion")); + fireEvent.change(screen.getByTestId("ac-style-0"), { target: { value: "checklist" } }); + // Converting an empty criterion seeds one empty item. + fireEvent.change(screen.getByLabelText("AC1 item 1"), { target: { value: "Email validates" } }); + fireEvent.click(screen.getByTestId("ac-checklist-add-AC1")); + fireEvent.change(screen.getByLabelText("AC1 item 2"), { target: { value: "Password masked" } }); + fireEvent.click(screen.getByLabelText("AC1 item 2 done")); + expect(screen.getByLabelText("AC1 item 1")).toHaveValue("Email validates"); + expect(screen.getByLabelText("AC1 item 2 done")).toBeChecked(); + }); + + it("renders a loaded checklist criterion in the checklist editor", () => { + render( + , + ); + expect(screen.getByTestId("ac-style-0")).toHaveValue("checklist"); + expect(screen.getByLabelText("AC1 item 1")).toHaveValue("a"); + expect(screen.getByLabelText("AC1 item 2 done")).toBeChecked(); + }); + + it("keeps a requirement link when the criterion style changes", () => { + render( + , + ); + expect(screen.getByTestId("ac-linked-req-0-R1")).toBeInTheDocument(); + fireEvent.change(screen.getByTestId("ac-style-0"), { target: { value: "checklist" } }); + // R# linking is unaffected by the style switch. + expect(screen.getByTestId("ac-linked-req-0-R1")).toBeInTheDocument(); + fireEvent.change(screen.getByTestId("ac-style-0"), { target: { value: "assertion" } }); + expect(screen.getByTestId("ac-linked-req-0-R1")).toBeInTheDocument(); + }); + + it("links an acceptance criterion to a requirement via the dropdown, not free text", () => { + render(); + fireEvent.click(screen.getByTestId("guided-add-acceptance-criterion")); + + // No free-text "(R#)" entry point exists — only the link dropdown. + expect(screen.queryByLabelText(/req_refs/i)).not.toBeInTheDocument(); + + const select = screen.getByTestId("ac-link-requirement-0"); + fireEvent.change(select, { target: { value: "R1" } }); + + expect(screen.getByTestId("ac-linked-req-0-R1")).toHaveTextContent("R1"); + // Once linked, R1 is no longer offered again in the dropdown. + expect(screen.queryByRole("option", { name: /R1/ })).not.toBeInTheDocument(); + }); + + it("unlinks a requirement from an acceptance criterion", () => { + render( + , + ); + expect(screen.getByTestId("ac-linked-req-0-R1")).toBeInTheDocument(); + fireEvent.click(screen.getByLabelText("Unlink R1 from AC1")); + expect(screen.queryByTestId("ac-linked-req-0-R1")).not.toBeInTheDocument(); + }); + + it("keeps Advanced collapsed by default and reveals it on toggle", () => { + render(); + expect(screen.queryByTestId("guided-advanced-panel")).not.toBeInTheDocument(); + fireEvent.click(screen.getByTestId("guided-advanced-toggle")); + expect(screen.getByTestId("guided-advanced-panel")).toBeInTheDocument(); + expect(screen.getByTestId("guided-execution-mode")).toBeInTheDocument(); + expect(screen.getByTestId("guided-constitution-refs")).toBeInTheDocument(); + expect(screen.getByTestId("guided-repos")).toBeInTheDocument(); + expect(screen.getByTestId("guided-decisions")).toBeInTheDocument(); + }); + + it("surfaces validation gaps as non-blocking nudges", () => { + render(); + expect(screen.getByTestId("guided-nudge-uncovered-R1")).toBeInTheDocument(); + // Nudges never disable Save; that's governed by `dirty`, not nudge count. + expect(screen.getByTestId("guided-save")).toBeEnabled(); + }); + + it("clears the coverage nudge once every requirement is linked", () => { + render( + , + ); + expect(screen.queryByTestId("guided-nudge-uncovered-R1")).not.toBeInTheDocument(); + }); + + it("shows a Ready-to-create checklist and coverage meter", () => { + render(); + expect(screen.getByTestId("guided-coverage-meter")).toHaveTextContent("0/1 requirements covered (0%)"); + expect(screen.getByTestId("checklist-item-goal")).toBeInTheDocument(); + expect(screen.getByTestId("checklist-item-coverage")).toBeInTheDocument(); + }); + + it("updates the coverage meter as requirements get linked", () => { + render( + , + ); + expect(screen.getByTestId("guided-coverage-meter")).toHaveTextContent( + "1/1 requirements covered (100%)", + ); + }); +}); diff --git a/apps/web/src/components/spec-studio/guided-mode.tsx b/apps/web/src/components/spec-studio/guided-mode.tsx new file mode 100644 index 00000000..7008fcf9 --- /dev/null +++ b/apps/web/src/components/spec-studio/guided-mode.tsx @@ -0,0 +1,652 @@ +"use client"; + +import { ChevronDown, ChevronRight, Plus, Trash2 } from "lucide-react"; +import { useState } from "react"; + +import { Button } from "@/components/ui/button"; +import { + SPEC_STATUSES, + type ADR, + type AcceptanceCriterion, + type ExecutionMode, + type Requirement, + type SpecManifest, + type SpecStatus, +} from "@/lib/api/types"; + +import { + addAcceptanceCriterion, + addAdr, + addRequirement, + classifyCriterionStyle, + composeChecklist, + composeGivenWhenThen, + computeChecklist, + computeCoverage, + computeNudges, + convertCriterionText, + type CriterionStyle, + parseChecklist, + parseGivenWhenThen, +} from "./guided-helpers"; + +export interface GuidedModeProps { + /** The current draft manifest (controlled). */ + value: SpecManifest; + onChange: (next: SpecManifest) => void; + onSave: () => void; + saving?: boolean; + dirty?: boolean; + saveError?: string | null; +} + +const EXECUTION_MODES: { value: ExecutionMode; label: string }[] = [ + { value: "single_agent", label: "Single agent" }, + { value: "supervised_multi_agent", label: "Supervised swarm" }, +]; + +/** + * The Guided mode — a friendly, structured form over the same `SpecManifest` + * the Markdown and YAML modes edit. Blocks: **Goal**, **Requirements**, + * **Acceptance Criteria** (Given/When/Then, linked to requirements via a + * dropdown — never by typing `(R#)`), **Constraints**, and a collapsed + * **Advanced** section (constitution refs, execution mode, repos, ADRs). + * Validation is surfaced as non-blocking nudges, alongside a Ready-to-create + * checklist and a requirement-coverage meter. + */ +const CRITERION_STYLES: { value: CriterionStyle; label: string }[] = [ + { value: "gherkin", label: "Given/When/Then" }, + { value: "assertion", label: "Plain assertion" }, + { value: "checklist", label: "Checklist" }, +]; + +export function GuidedMode({ value, onChange, onSave, saving = false, dirty = false, saveError }: GuidedModeProps) { + const [advancedOpen, setAdvancedOpen] = useState(false); + // Style is derived from each criterion's text, but an explicit pick (keyed by + // criterion id) wins so an empty "assertion" doesn't snap back to the Gherkin + // default. Editing the text never clears the pick — R# links are unaffected. + const [styleOverrides, setStyleOverrides] = useState>({}); + const requirements = value.requirements ?? []; + const criteria = value.acceptance_criteria ?? []; + const constraints = value.constraints ?? []; + const constitutionRefs = value.constitution_refs ?? []; + const repos = value.repos ?? []; + const decisions = value.decisions ?? []; + + const nudges = computeNudges(value); + const checklist = computeChecklist(value); + const coverage = computeCoverage(value); + + function setRequirements(next: Requirement[]) { + onChange({ ...value, requirements: next }); + } + + function setCriteria(next: AcceptanceCriterion[]) { + onChange({ ...value, acceptance_criteria: next }); + } + + function setDecisions(next: ADR[]) { + onChange({ ...value, decisions: next }); + } + + return ( +
+
+ + {dirty ? "Unsaved changes" : "Guided"} + + +
+ + + + + + {/* --- Requirements ------------------------------------------------- */} +
+

+ Requirements +

+
    + {requirements.map((req, index) => ( +
  • + + {req.id} + + { + const next = [...requirements]; + next[index] = { ...next[index], text: event.target.value }; + setRequirements(next); + }} + className="flex-1 rounded-md border border-border bg-card px-3 py-1.5 text-sm text-foreground outline-none" + /> + +
    • + ))} +
+ +
+ + {/* --- Acceptance Criteria ------------------------------------------ */} +
+

+ Acceptance Criteria +

+
    + {criteria.map((ac, index) => { + const refs = ac.req_refs ?? []; + const linkable = requirements.filter((r) => !refs.includes(r.id)); + const style = styleOverrides[ac.id] ?? classifyCriterionStyle(ac.text); + + function setText(text: string) { + const next = [...criteria]; + next[index] = { ...next[index], text }; + setCriteria(next); + } + + function changeStyle(nextStyle: CriterionStyle) { + setStyleOverrides((prev) => ({ ...prev, [ac.id]: nextStyle })); + setText(convertCriterionText(ac.text, nextStyle)); + } + + return ( +
  • +
    + + {ac.id} + + + +
    + + {style === "gherkin" ? ( + + ) : style === "checklist" ? ( + + ) : ( + + )} + +
    + {refs.map((refId) => ( + + {refId} + + + ))} + {linkable.length > 0 ? ( + + ) : null} +
    +
    • + ); + })} +
+ +
+ + {/* --- Constraints ---------------------------------------------------- */} +
+

+ Constraints +

+
    + {constraints.map((constraint, index) => ( +
  • + { + const next = [...constraints]; + next[index] = event.target.value; + onChange({ ...value, constraints: next }); + }} + className="flex-1 rounded-md border border-border bg-card px-3 py-1.5 text-sm text-foreground outline-none" + /> + +
    • + ))} +
+ +
+ + {/* --- Advanced (collapsed by default) -------------------------------- */} +
+ + {advancedOpen ? ( +
+ + + onChange({ ...value, constitution_refs: next })} + /> + + onChange({ ...value, repos: next })} + /> + +
+ Architecture decisions +
    + {decisions.map((adr, index) => ( +
  • + + {adr.id} + + { + const next = [...decisions]; + next[index] = { ...next[index], title: event.target.value }; + setDecisions(next); + }} + className="flex-1 rounded-md border border-border bg-card px-3 py-1.5 text-sm text-foreground outline-none" + placeholder="Decision title" + /> + +
    • + ))} +
+ +
+
+ ) : null} +
+ + {/* --- Nudges ---------------------------------------------------------- */} + {nudges.length > 0 ? ( +
    + {nudges.map((nudge) => ( +
  • + {nudge.message} +
    • + ))} +
+ ) : null} + + {/* --- Ready-to-create checklist + coverage meter ---------------------- */} +
+
+

+ Ready to create +

+ + {coverage.satisfied}/{coverage.total} requirements covered ({coverage.pct}%) + +
+
+
= 100 ? "bg-success" : "bg-primary"}`} + style={{ width: `${coverage.pct}%` }} + /> +
+
    + {checklist.map((item) => ( +
  • + + + {item.label} + +
    • + ))} +
+
+ + {saveError ? ( +

+ {saveError} +

+ ) : null} +
+ ); +} + +function StringListField({ + label, + testId, + addTestId, + items, + onChange, +}: { + label: string; + testId: string; + addTestId: string; + items: string[]; + onChange: (next: string[]) => void; +}) { + return ( +
+ {label} +
    + {items.map((item, index) => ( +
  • + { + const next = [...items]; + next[index] = event.target.value; + onChange(next); + }} + className="flex-1 rounded-md border border-border bg-card px-3 py-1.5 text-sm text-foreground outline-none" + /> + +
    • + ))} +
+ +
+ ); +} + +/** Given/When/Then editor — three inputs composing the criterion's `text`. */ +function GherkinEditor({ id, text, onChange }: { id: string; text: string; onChange: (text: string) => void }) { + const gwt = parseGivenWhenThen(text); + const update = (patch: Partial) => onChange(composeGivenWhenThen({ ...gwt, ...patch })); + return ( +
+ {(["given", "when", "then"] as const).map((clause) => ( + + ))} +
+ ); +} + +/** Checklist editor — a togglable, editable list of check items in `text`. */ +function ChecklistEditor({ id, text, onChange }: { id: string; text: string; onChange: (text: string) => void }) { + const items = parseChecklist(text); + const commit = (next: typeof items) => onChange(composeChecklist(next)); + return ( +
+
    + {items.map((item, index) => ( +
  • + + commit(items.map((it, i) => (i === index ? { ...it, checked: event.target.checked } : it))) + } + className="h-4 w-4 shrink-0 accent-primary" + /> + + commit(items.map((it, i) => (i === index ? { ...it, label: event.target.value } : it))) + } + placeholder="Checklist item" + className="flex-1 rounded-md border border-border bg-card px-2 py-1.5 text-sm text-foreground outline-none" + /> + +
    • + ))} +
+ +
+ ); +} diff --git a/apps/web/src/components/spec-studio/markdown-mode.test.tsx b/apps/web/src/components/spec-studio/markdown-mode.test.tsx new file mode 100644 index 00000000..148d3f93 --- /dev/null +++ b/apps/web/src/components/spec-studio/markdown-mode.test.tsx @@ -0,0 +1,136 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { useState } from "react"; +import { describe, expect, it, vi } from "vitest"; + +import { MarkdownMode } from "./markdown-mode"; + +const VALID = `--- +id: SPEC-1 +status: draft +constitution_refs: [] +repos: [] +execution_mode: single_agent +skill_profile: null +plan_ref: null +tasks_ref: null +validation_ref: null +--- + +## Goal + +Passwordless auth + +## Requirements + +- **R1**: Users can sign in without a password + +## Acceptance Criteria + +- **AC1** (R1): Given a valid magic link, when clicked, then the user is signed in + +## Constraints + +- Must work offline +`; + +function Harness({ initial }: { initial: string }) { + const [value, setValue] = useState(initial); + return ; +} + +describe("MarkdownMode", () => { + it("shows a valid parse status and defaults to the Structure panel", () => { + render(); + expect(screen.getByTestId("markdown-status-valid")).toBeInTheDocument(); + expect(screen.getByTestId("markdown-panel-structure")).toBeInTheDocument(); + expect(screen.getByTestId("markdown-save")).toBeDisabled(); + }); + + it("renders frontmatter and body verbatim in the raw textarea", () => { + render(); + const textarea = screen.getByTestId("markdown-textarea"); + expect(textarea).toHaveValue(VALID); + expect((textarea as HTMLTextAreaElement).value).toContain("---\nid: SPEC-1"); + }); + + it("renders a line-number gutter matching the text line count", () => { + const { container } = render(); + const gutterLines = container.querySelectorAll('[aria-hidden="true"] > div'); + expect(gutterLines.length).toBeGreaterThanOrEqual(VALID.split("\n").length - 1); + }); + + it("switches between Structure, Preview and Traceability panels", () => { + render(); + + fireEvent.click(screen.getByTestId("markdown-panel-tab-preview")); + expect(screen.getByTestId("markdown-panel-preview")).toBeInTheDocument(); + expect(screen.getByText("Passwordless auth")).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId("markdown-panel-tab-traceability")); + expect(screen.getByTestId("markdown-panel-traceability")).toBeInTheDocument(); + expect(screen.getByTestId("traceability-matrix")).toBeInTheDocument(); + + fireEvent.click(screen.getByTestId("markdown-panel-tab-structure")); + expect(screen.getByTestId("markdown-panel-structure")).toBeInTheDocument(); + }); + + it("shows line-anchored parse issues for malformed markdown and disables save on invalid content is independent of dirty", () => { + render( + , + ); + expect(screen.getByTestId("markdown-status-invalid")).toBeInTheDocument(); + const issues = screen.getByTestId("markdown-issues"); + expect(issues).toBeInTheDocument(); + expect(screen.getByText(/requirement must be/i)).toBeInTheDocument(); + expect(screen.getByText(/missing a '## Goal' section/i)).toBeInTheDocument(); + }); + + it("clicking an issue jumps the textarea cursor to that line", () => { + const text = "---\nid: SPEC-1\n---\n\n## Goal\n\nX\n\n## Requirements\n\n- broken\n"; + render(); + const textarea = screen.getByTestId("markdown-textarea") as HTMLTextAreaElement; + const issueButton = screen.getByText(/requirement must be/i).closest("button")!; + fireEvent.click(issueButton); + expect(document.activeElement).toBe(textarea); + }); + + it("live-updates the parse status and structure counts as the user types", () => { + render(); + expect(screen.getByTestId("markdown-status-invalid")).toBeInTheDocument(); + + const textarea = screen.getByTestId("markdown-textarea"); + fireEvent.change(textarea, { target: { value: VALID } }); + + expect(screen.getByTestId("markdown-status-valid")).toBeInTheDocument(); + fireEvent.click(screen.getByTestId("markdown-panel-tab-structure")); + expect(screen.getByText("Requirements")).toBeInTheDocument(); + }); + + it("Tab inserts an indent instead of moving focus out of the editor", () => { + render(); + const textarea = screen.getByTestId("markdown-textarea") as HTMLTextAreaElement; + textarea.focus(); + textarea.setSelectionRange(3, 3); + fireEvent.keyDown(textarea, { key: "Tab" }); + expect(textarea).toHaveValue("abc "); + }); + + it("enables save once dirty and calls onSave", () => { + const onSave = vi.fn(); + render(); + const button = screen.getByTestId("markdown-save"); + expect(button).toBeEnabled(); + fireEvent.click(button); + expect(onSave).toHaveBeenCalled(); + }); + + it("surfaces a save error when provided", () => { + render(); + expect(screen.getByTestId("markdown-save-error")).toHaveTextContent("409 conflict"); + }); +}); diff --git a/apps/web/src/components/spec-studio/markdown-mode.tsx b/apps/web/src/components/spec-studio/markdown-mode.tsx new file mode 100644 index 00000000..62d86a6a --- /dev/null +++ b/apps/web/src/components/spec-studio/markdown-mode.tsx @@ -0,0 +1,378 @@ +"use client"; + +import { AlertTriangle, CheckCircle2, ListTree, Eye as EyeIcon, Route } from "lucide-react"; +import { useMemo, useRef, useState, type KeyboardEvent, type UIEvent } from "react"; + +import { Button } from "@/components/ui/button"; +import { TraceabilityMatrix } from "@/components/spec/traceability-matrix"; +import { computeChecklist, computeCoverage, computeNudges } from "@/components/spec-studio/guided-helpers"; +import { hasMarkdownErrors, parseSpecMarkdown, type MarkdownIssue } from "@/lib/spec-studio/markdown-parse"; +import type { RequirementTrace } from "@/lib/api/types"; +import { cn } from "@/lib/utils"; + +export interface MarkdownModeProps { + /** The current `spec.md` text (controlled). */ + value: string; + onChange: (next: string) => void; + onSave: () => void; + saving?: boolean; + dirty?: boolean; + saveError?: string | null; +} + +type PanelTab = "structure" | "preview" | "traceability"; + +const PANEL_TABS: { id: PanelTab; label: string; icon: typeof ListTree }[] = [ + { id: "structure", label: "Structure", icon: ListTree }, + { id: "preview", label: "Preview", icon: EyeIcon }, + { id: "traceability", label: "Traceability", icon: Route }, +]; + +function jumpToLine(textareaRef: React.RefObject, line: number) { + const textarea = textareaRef.current; + if (!textarea) return; + const lines = textarea.value.split("\n"); + let offset = 0; + for (let i = 0; i < line - 1 && i < lines.length; i += 1) { + offset += lines[i].length + 1; + } + textarea.focus(); + textarea.setSelectionRange(offset, offset); +} + +/** Build a lightweight, local requirement traceability from the parsed markdown alone (no task/test refs — those come from a backend validation run). */ +function localTraces( + manifest: ReturnType["manifest"], +): RequirementTrace[] { + const requirements = manifest.requirements ?? []; + const criteria = manifest.acceptance_criteria ?? []; + return requirements.map((req) => { + const linked = criteria.filter((ac) => (ac.req_refs ?? []).includes(req.id)).map((ac) => ac.id); + return { + requirement_id: req.id, + text: req.text, + acceptance_criteria_ids: linked, + task_refs: [], + test_refs: [], + satisfied: linked.length > 0, + }; + }); +} + +function IssueList({ + issues, + onJump, +}: { + issues: MarkdownIssue[]; + onJump: (line: number) => void; +}) { + if (issues.length === 0) return null; + return ( +
    + {issues.map((issue, index) => ( +
  • + +
    • + ))} +
+ ); +} + +function StructurePanel({ manifest }: { manifest: ReturnType["manifest"] }) { + const checklist = computeChecklist(manifest); + const nudges = computeNudges(manifest); + const coverage = computeCoverage(manifest); + const counts: { label: string; count: number }[] = [ + { label: "Requirements", count: (manifest.requirements ?? []).length }, + { label: "Acceptance criteria", count: (manifest.acceptance_criteria ?? []).length }, + { label: "Constraints", count: (manifest.constraints ?? []).length }, + { label: "Open questions", count: (manifest.open_questions ?? []).length }, + { label: "Decisions", count: (manifest.decisions ?? []).length }, + ]; + return ( +
+
+ {counts.map((c) => ( +
+
{c.label}
+
{c.count}
+
+ ))} +
+
+

+ Requirement coverage: {coverage.satisfied}/{coverage.total} ({coverage.pct}%) +

+
    + {checklist.map((item) => ( +
  • + + {item.label} +
    • + ))} +
+
+ {nudges.length > 0 ? ( +
    + {nudges.map((nudge) => ( +
  • + + {nudge.message} +
    • + ))} +
+ ) : null} +
+ ); +} + +function PreviewPanel({ manifest }: { manifest: ReturnType["manifest"] }) { + const requirements = manifest.requirements ?? []; + const criteria = manifest.acceptance_criteria ?? []; + const constraints = manifest.constraints ?? []; + const openQuestions = manifest.open_questions ?? []; + const decisions = manifest.decisions ?? []; + return ( +
+
+

Goal

+

{manifest.name || "—"}

+
+ {requirements.length > 0 ? ( +
+

Requirements

+
    + {requirements.map((r) => ( +
  • + {r.id} {r.text} +
    • + ))} +
+
+ ) : null} + {criteria.length > 0 ? ( +
+

Acceptance Criteria

+
    + {criteria.map((c) => ( +
  • + {c.id}{" "} + {(c.req_refs ?? []).length > 0 ? ( + + ({(c.req_refs ?? []).join(", ")}) + + ) : null}{" "} + {c.text} +
    • + ))} +
+
+ ) : null} + {constraints.length > 0 ? ( +
+

Constraints

+
    + {constraints.map((c, i) => ( +
  • + {c} +
    • + ))} +
+
+ ) : null} + {openQuestions.length > 0 ? ( +
+

Open Questions

+
    + {openQuestions.map((q) => ( +
  • + {q.id} {q.text} + {q.resolution ? ( + Resolution: {q.resolution} + ) : null} +
    • + ))} +
+
+ ) : null} + {decisions.length > 0 ? ( +
+

Decisions

+
    + {decisions.map((d) => ( +
  • + {d.id} — {d.title} +
    • + ))} +
+
+ ) : null} +
+ ); +} + +/** + * The `spec.md` prose editor — Spec Studio's default human/agent surface. + * A JetBrains Mono, keyboard-first raw-text editor (frontmatter always + * visible, `Tab` inserts an indent instead of leaving the field, a + * line-number gutter) paired with a live parsed pane — **Structure** + * (section counts, the Ready checklist and coverage meter), + * **Preview** (a readable render of the parsed sections) and + * **Traceability** (local requirement -> acceptance-criteria coverage) — plus + * a line-anchored parse-issue list, all recomputed on every keystroke via + * `parseSpecMarkdown` (a client-side mirror of `forge_spec.markdown.parse_spec_md`). + * Saving re-renders `manifest.yaml` to match on the backend, which remains + * the authoritative parser. + */ +export function MarkdownMode({ + value, + onChange, + onSave, + saving = false, + dirty = false, + saveError, +}: MarkdownModeProps) { + const [panel, setPanel] = useState("structure"); + const textareaRef = useRef(null); + const gutterRef = useRef(null); + const [scrollTop, setScrollTop] = useState(0); + + const { manifest, issues } = useMemo(() => parseSpecMarkdown(value), [value]); + const invalid = hasMarkdownErrors(issues); + const lineCount = useMemo(() => Math.max(1, value.split("\n").length), [value]); + const traces = useMemo(() => localTraces(manifest), [manifest]); + + const onScroll = (event: UIEvent) => { + setScrollTop(event.currentTarget.scrollTop); + }; + + /** Keyboard-first: `Tab` indents (two spaces) instead of leaving the editor. */ + const onKeyDown = (event: KeyboardEvent) => { + if (event.key !== "Tab") return; + event.preventDefault(); + const textarea = event.currentTarget; + const { selectionStart, selectionEnd } = textarea; + const next = `${value.slice(0, selectionStart)} ${value.slice(selectionEnd)}`; + onChange(next); + requestAnimationFrame(() => { + textarea.setSelectionRange(selectionStart + 2, selectionStart + 2); + }); + }; + + return ( +
+
+
+ {invalid ? ( + + + {issues.filter((i) => i.severity === "error").length} issue + {issues.filter((i) => i.severity === "error").length === 1 ? "" : "s"} + + ) : ( + + + Parses cleanly + + )} + {dirty ? Unsaved changes : null} +
+ +
+ +
+
+
+ {Array.from({ length: lineCount }, (_, i) => ( +
{i + 1}
+ ))} +
+