diff --git a/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/plan.md b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/plan.md new file mode 100644 index 0000000..314bbbd --- /dev/null +++ b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/plan.md @@ -0,0 +1,125 @@ +# Plan — SPEC-018 + +## Existing state + +- `internal/workflow/workflow.go` owns `All()`, `transitions`, `Next()`, + `Check()` and `WaitingFor()`. It has no `Meaning`: the one-line meaning of + each state lives only in `kit/machine/WORKFLOW.md`, which is the drift the + spec names. `WaitingFor(Blocked)` returns `"conductor: clear the blocker"` + (line 143), the one place the role shows up in code. +- `internal/cli/machine.go:cmdWorkflow` prints `kit.Workflow()` verbatim + (lines 13–24); `cmdRoles` prints `kit.Roles()` (27–43). Neither renders. +- `kit/machine.go` serves `Workflow()`, `Roles()`, `Role()`, `Template()` + from the embedded `kit/machine/` tree. `kit/machine_test.go` already pins + `Roles()` to `architect,implementer,orchestrator,reviewer`. +- `kit/machine/WORKFLOW.md` carries the hand-written `## States` table whose + `blocked` row says `conductor`; it is the second copy. +- `internal/project/project.go`: `Spec.Conductor` (line 64), `FromDoc` reads + `d.Str("conductor")` (505), `Save` writes it with `setOrDelete` (460). + `Criteria` falls back to `Criterios de aceptación` (360), `Contract` to + `Contrato` (376), `OpenQuestions` to `Preguntas abiertas` (385). +- `internal/cli/work.go`: `cmdStart` sets `s.Conductor` (179–196) and the + `--by` help; `pendingConventions` reads both `Proposed conventions` and + `Convenciones propuestas` (490). +- `internal/view/view.go` prints `conductor` (271). +- `internal/cli/cli_test.go` runs commands end to end (`newRepo`, `run`, + `mustRun`, `write`, `read`); `internal/cli/machine_test.go` asserts the + workflow output contains `## States`. These are the homes for the new + checks. +- Docs that restate the machine: `docs/workflow.md` (loop lines 5–10, + States table 16–29), `docs/teams.md` (conductor role paragraph, line 88), + `docs/cli.md:62` (`forge start` "creates `plan.md` and `tasks.md`"), + `docs/customizing.md:55-56` (claims both heading languages are accepted); + loop blocks in `AGENTS.md:22-23`, `kit/AGENTS.md:21-22`, `README.md:74-75`. + `kit/claude/skills/forge-work/SKILL.md` (and the planted `.claude/` copy) + already say `orchestrator` and carry no state list, so it needs no edit. +- Conventions that apply: `.forge/conventions/testing.md` (add a seam only + where a test must substitute; these tests need none), `cli-output.md` + (notices are `warning: ` lines; errors are returned), `frontmatter.md` + (list fields normalise through `normalizeIDs`, empty removes the key). + Decision 0002's release step still names the conductor and is updated by + this spec; the other `.forge/decisions/` and delivered specs are history + and are not rewritten. +- Genuinely new: `workflow.Meaning`, the `` renderer, + the `orchestrator` frontmatter key with its read-only `conductor` fallback, + and the tests that hold the Markdown to the binary. + +## Phase 1 — Workflow states: one declaration, rendered + +- Scope: add `Meaning(State) string` to `internal/workflow/workflow.go` and + change `WaitingFor(Blocked)` to `"orchestrator: clear the blocker"`. In + `kit/machine/WORKFLOW.md` replace the table body with a `## States` heading + and the `` marker. In `internal/cli/machine.go` make + `cmdWorkflow` render the marker into a table from `workflow.All()`, + `Meaning()` and `WaitingFor()` (a small `renderWorkflow` the internal test + can call); a missing marker is an error naming `kit/machine/WORKFLOW.md`. +- Done when: `forge workflow` prints the table generated from Go and the + Markdown no longer holds a second table. Moves AC1, and the `blocked` row + of AC2. +- Verify with: `go test ./...`, `gofmt -l .`, `go vet ./...`, and + `go run . workflow` by eye. + +## Phase 2 — Pages stop restating; `docs/cli` matches behaviour + +- Scope: delete the loop diagram and `## States` table from + `docs/workflow.md` and have it point at `forge workflow`; delete the loop + block from `AGENTS.md`, `kit/AGENTS.md` and `README.md`. Correct the + `forge start` paragraph in `docs/cli.md` (no `plan.md`/`tasks.md` at + start; planning writes them after approval). Add the `cli_test.go` + scanning checks: no `state → state` chain and no `| \`state\` |` row in + `AGENTS.md`, `kit/AGENTS.md`, `README.md` or `docs/*.md`; `docs/workflow.md` + names `forge workflow`; `docs/cli.md` drops the old claim. +- Done when: no page carries an ordered state list, the docs link to the + binary, and `docs/cli.md` describes `forge start` as it behaves. Moves AC1, + AC3, AC5. +- Verify with: `go test ./...`, `gofmt -l .`, `go vet ./...`. + +## Phase 3 — One name for the driver: `orchestrator` + +- Scope: rename `project.Spec.Conductor` to `Spec.Orchestrator`; `FromDoc` + reads `orchestrator` and falls back to `conductor`; `Save` writes + `orchestrator` and deletes the legacy `conductor` key. Update + `cmdStart` in `internal/cli/work.go` (assignment and `--by` help) and + `internal/view/view.go` (`Brief`, `Detail`). Update `docs/teams.md`, + `docs/cli.md` and the release step in `AGENTS.md` to say orchestrator; + leave `.forge/` records as written. Tests: legacy `conductor` loads and the + next save rewrites it (`internal/project/project_test.go`), `Detail` prints + `orchestrator` (`internal/view/view_test.go`), and after + `start --by ana` the spec has `orchestrator:` and no `conductor:` + (`internal/cli/cli_test.go`), plus no `conductor` in the scanned pages. +- Done when: frontmatter, help and docs use `orchestrator` only, and old + specs still read. Moves AC2. +- Verify with: `go test ./...`, `gofmt -l .`, `go vet ./...`, and + `go run . status SPEC-018`. + +## Phase 4 — English section headings only + +- Scope: drop the Spanish fallbacks in `project.Criteria`, `project.Contract` + and `project.OpenQuestions` (`internal/project/project.go`) and the + `Convenciones propuestas` heading in `internal/cli/work.go` + (`pendingConventions`). Correct the working-language paragraph in + `docs/customizing.md`. Tests: a Spanish-only body yields empty criteria, + contract and questions (`internal/project/project_test.go`); a proposal + under `Convenciones propuestas` no longer blocks archive while one under + `Proposed conventions` does, and `docs/customizing.md` no longer claims + both (`internal/cli/cli_test.go`). +- Done when: the CLI reads English headings only and the docs say so. Moves + AC4. +- Verify with: `go test ./...`, `gofmt -l .`, `go vet ./...`. + +## Risks + +- Decision 4 renames a frontmatter key. The fallback must stay read-only and + `Save` must delete `conductor`, or a legacy spec that is saved again would + carry both keys and fail the single-name check. If a host tool reads + `conductor` directly, it must move to `orchestrator`; this is the one call + a reviewer could reasonably flip. +- `internal/cli/machine_test.go` asserts the output contains `## States`. + Keeping the heading while rendering its body keeps that test meaningful; + deleting the heading would make it silently weaker. +- `docs/workflow.md` is hand-written and long. Removing the table must not + remove the prose that explains open questions, coverage, dependencies or + drift. +- Scope creep: SPEC-015 and SPEC-016 change states and add a fast lane inside + `internal/workflow`; this spec must not. Do not rename `specifying` and do + not add a flag here. diff --git a/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/review.md b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/review.md new file mode 100644 index 0000000..650cbf8 --- /dev/null +++ b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/review.md @@ -0,0 +1,29 @@ +# Review — SPEC-018 + +Verdict: pass with notes + +## Acceptance criteria + +| Criterion | Result | Evidence | +|---|---|---| +| AC1 | pass | `internal/workflow.All()` is the single ordered declaration, and `renderWorkflow` (`internal/cli/machine.go:46-55`) builds the table by iterating `workflow.All()` and calling `Meaning()`/`WaitingFor()`. `kit/machine/WORKFLOW.md:10` holds only ``, no hand-written table. `go run . workflow` prints the ten states in lifecycle order. In an isolated copy, adding `State("probing")` to `All()` made `go run . workflow` print a `probing` row; appending a `proposed -> accepted` arrow chain to `AGENTS.md` made `TestDocs_DoNotRestateTheStateMachine` fail with "restates the state machine as an arrow chain". `kit.Roles()` derives from `kit/machine/roles/*.md`; `TestWorkflow_StatesAreRenderedFromTheWorkflowPackage` fails if any `WaitingFor` token is not `anyone`, `nobody` or a `kit.Roles()` member, and pins `architect,implementer,orchestrator,reviewer`. Tests: `TestWorkflowCommand_RendersTheStatesFromGo` (two runs byte-identical, rows in `All()` order), `TestRenderWorkflow_MissingMarkerIsAnError`, `TestMeaning_EveryStateHasALine`. | +| AC2 | pass | `orchestratorFrom` (`internal/project/project.go:522-529`) reads `orchestrator` then the read-only legacy `conductor`; `Save` sets `orchestrator` and `doc.Delete("conductor")` (455-456). `go run . status SPEC-001` prints `orchestrator carra` while `.forge/specs/SPEC-001-.../spec.md` still carries `conductor: carra`; `go run . status SPEC-018` prints `orchestrator TheJisus28`, and commit `060ec2a` shows the advance rewrote the key. `go run . roles` lists `architect,implementer,orchestrator,reviewer`; `go run . start --help` says `who orchestrates this spec`. `rg -i conductor` over `AGENTS.md`, `kit/AGENTS.md`, `README.md`, `kit/machine/WORKFLOW.md` and `docs/*.md` finds nothing (only the fallback and test scaffolding remain). Tests: `TestSave_MigratesLegacyConductorKey`, `TestStart_RecordsOneOrchestratorName`, `TestDetail_ShowsOrchestrator`, `TestWaitingFor_BlockedNamesTheOrchestrator`, `TestDocs_DoNotSayConductor`. | +| AC3 | pass | `docs/cli.md:57-64` says `forge start` creates the spec folder and records fingerprints, and "does not create `plan.md` or `tasks.md`". End-to-end in a scratch repo: after `init`, `new`, `accept --by jesus`, `start --by ana`, the spec folder holds only `spec.md`, frontmatter is `orchestrator: ana`, and `status` prints `orchestrator ana`. `cmdStart` (`internal/cli/work.go:165-202`) does only `os.MkdirAll` + `Save`. Tests: `TestDocs_ForgeStartDoesNotCreatePlanningFiles`. | +| AC4 | pass | `project.Criteria`/`Contract`/`OpenQuestions` read only `Acceptance criteria`/`Contract`/`Open questions` (`internal/project/project.go:360,373,380`); `pendingConventions` reads only `Proposed conventions` (`internal/cli/work.go:492`). `docs/customizing.md:51-57` names the five fixed English headings and says a translated heading is not known. Tests: `TestSpec_ReadsEnglishSectionHeadingsOnly`, `TestArchive_ReadsOnlyEnglishConventionHeading` (English blocks, Spanish invisible), `TestDocs_CustomizingDoesNotAcceptTranslatedHeadings`. | +| AC5 | pass | `docs/workflow.md:5-8` points at `forge workflow` for the states and transitions and keeps only the explanation (open questions, criteria, hierarchy, coverage, dependencies, drift). No state arrow chain and no table row whose first cell is a backticked state name in `docs/*.md`, `AGENTS.md`, `kit/AGENTS.md`, `README.md`; the only arrow left is the Conventional Commits `fix -> patch` line at `AGENTS.md:53`. Tests: `TestDocs_DoNotRestateTheStateMachine`, `TestDocs_WorkflowPointsAtForgeWorkflow`. | + +Full gate: `go test ./...` all packages `ok`; `gofmt -l .` and `go vet ./...` no output; `go run . validate` passed after this file was written. + +## Blocking problems + +None. + +## Notes + +- The approved contract's `Tests` line for `internal/cli/cli_test.go` also promised "the spec folder holds only `spec.md` (`plan.md` and `tasks.md` are absent)" after `new`/`accept`/`start`. `TestStart_RecordsOneOrchestratorName` checks the frontmatter key and `forge status` but not the folder contents, and no other test asserts the absence. The behaviour is correct (verified end to end above), so this is a test-coverage gap against the contract, not an AC failure. Reproduce: `go test ./internal/cli/ -run TestStart_RecordsOneOrchestratorName -v` passes while holding no folder-contents assertion. +- `TestDocs_DoNotRestateTheStateMachine` builds its regex from the current `workflow.All()` names, so it catches a page that lists the current states but would miss a stale list of names that a future rename (SPEC-015) removes. Out of scope here, worth revisiting when states are renamed. +- The `conductor` to `orchestrator` save migration is visible in the spec's own transition commit `060ec2a`, which rewrote `conductor: TheJisus28` to `orchestrator: TheJisus28` without touching `contract_hash` (`ec06b4bfa420`), so there is no contract drift. + +## Proposed conventions + +None. \ No newline at end of file diff --git a/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/spec.md b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/spec.md index 1de4389..196df5f 100644 --- a/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/spec.md +++ b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/spec.md @@ -1,10 +1,17 @@ --- id: SPEC-018 title: Keep one source of truth for the workflow and roles -status: proposed +status: done capability: workflow created: 2026-09-20 updated: 2026-09-20 +accepted_by: TheJisus28 +approved_by: TheJisus28 +contract_hash: ec06b4bfa420 +orchestrator: TheJisus28 +pr_url: "https://github.com/TheJisus28/forge/pull/21" +pr_state: open +pr: 21 --- ## Problem @@ -43,15 +50,166 @@ None. ## Contract -Written by the architect once the work is accepted, and frozen once -approved. Real names from this repository: modules, endpoints, -tables, screens. Numbered decisions with what they discard. Anything other -specs will build against goes here. +### Decisions + +**1. `internal/workflow` is the only declaration of the states and their +order; `forge workflow` renders the table from it.** +`internal/workflow/workflow.go` keeps `All()` and the `transitions` slice and +gains `Meaning(State) string`, the one-line meaning that today lives only in +the Markdown. `kit/machine/WORKFLOW.md` loses its hand-written `## States` +table and carries the marker `` where the table was; +`cmdWorkflow` in `internal/cli/machine.go` replaces that marker with a table +built from `workflow.All()`, `workflow.Meaning()` and +`workflow.WaitingFor()`. A missing marker is an error naming +`kit/machine/WORKFLOW.md`, so the source cannot silently lose the table. +Discards: the second copy in Markdown (it already drifted: `blocked` said +`conductor`) and any second state list in Go. + +**2. Every Markdown page stops restating the state list and points at `forge +workflow`.** The loop diagram and the `## States` table are deleted from +`docs/workflow.md`, and the loop line is deleted from `AGENTS.md`, +`kit/AGENTS.md` and `README.md`; each says the states and transitions live in +the binary. `docs/workflow.md` keeps what is explanation — open questions, +criteria, hierarchy, coverage, dependencies, drift — but no ordered state +list. The `kit/` copy is the source; `forge update` refreshes the planted +`.claude/` and `.opencode/` copies. Discards: keeping a copy in a page and +testing it for equality (a copy is a second source even when a test guards +it), and deleting `docs/workflow.md` entirely (the rationale is worth +keeping). + +**3. `kit/machine/roles/` is the only list of roles; `workflow.WaitingFor` +may only name a role from it.** `forge roles` already lists the role files, +so no role list is added to Go. `WaitingFor(Blocked)` changes from +`"conductor: clear the blocker"` to `"orchestrator: clear the blocker"`. A +test walks every state and fails when the word before `:` is not `anyone`, +`nobody`, or a member of `kit.Roles()`. Discards: a fifth `conductor` role +(the driver already exists as `orchestrator`) and a Go role-name constant +that could drift from the files. + +**4. The driver is `orchestrator` everywhere; the `conductor` frontmatter key +is retired.** `project.Spec.Conductor` becomes `Spec.Orchestrator`; +`project.Save` writes `orchestrator` and deletes a legacy `conductor`; +`FromDoc` reads `orchestrator` and falls back to `conductor`, so specs +written before this change keep their recorded actor. `cmdStart`'s `--by` +help in `internal/cli/work.go` says "who orchestrates this spec"; +`internal/view/view.go` prints `orchestrator` in `Brief` and `Detail`; +`docs/teams.md`, `docs/cli.md` and the release step in `AGENTS.md` say +orchestrator. Delivered `.forge/specs/` history and `.forge/decisions/` +records are left as written; decision 0002's rule is unchanged by the role's +name. Discards: keeping `conductor` as a second name (AC2 asks for one) and +renaming the existing `orchestrator` role instead (that would touch every +host adapter and leave old history lines contradicting the new name). +**Expensive, hard to reverse:** it renames a frontmatter key, so any tool +that reads `conductor` must move to `orchestrator`; the fallback keeps old +records readable until their next save, and no migration command is added. + +**5. `docs/cli.md` matches `forge start`; the command is not changed.** +`forge start` records the orchestrator and `@contract` fingerprints and does +not create `plan.md` or `tasks.md`; those are written during planning, after +`forge approve`. The `forge start` paragraph is corrected to say so. +Discards: making `forge start` create empty `plan.md`/`tasks.md` (artifacts +before approval, and `forge validate` expects them only at `implementing`). + +**6. The CLI reads English section headings only.** `project.Criteria`, +`project.Contract` and `project.OpenQuestions` stop falling back to +`Criterios de aceptación`, `Contrato` and `Preguntas abiertas`; +`pendingConventions` in `internal/cli/work.go` stops reading `Convenciones +propuestas`. `working_language` still governs the prose inside a spec, +decision or convention; only the `## ` headings are fixed English +(`Acceptance criteria`, `Contract`, `Open questions`, `Proposed +conventions`, `Existing state`). `docs/customizing.md` is corrected. +Discards: the Spanish heading aliases and the claim that both are accepted. +The `- ninguna` value `isNone` accepts is content, not a heading, and is +unchanged (SPEC-019, decision 2). + +### Interfaces other specs build against + +- `internal/workflow`: `All()`, `Next()`, `WaitingFor()` keep their + signatures; new `Meaning(State) string`. `WaitingFor` returns + `: ` (or `anyone`/`nobody`), and the role token is a name from + `kit.Roles()`. SPEC-015 (rename `specifying` → `contracting`) and SPEC-016 + (fast lane) change `All()`/`transitions`/`Meaning` in this one package; + `forge workflow` and the tests follow with no Markdown edit. +- `forge workflow`: prints `kit/machine/WORKFLOW.md` with `` replaced by the generated `## States` table. `kit.Workflow()` stays the + raw embedded copy; callers that need the rendered text go through + `cmdWorkflow` or the same renderer in `internal/cli/machine.go`. +- Roles: `kit.Roles()` — the `.md` files in `kit/machine/roles/` — is the + normative list; `forge roles` and the "who acts next" column of `forge + workflow` both derive from it. +- Frontmatter: the key is `orchestrator`; `conductor` is read-only legacy and + is never written. New readers use `orchestrator`. +- Sections: the CLI reads `Acceptance criteria`, `Contract`, `Open + questions`, `Proposed conventions` and `Existing state`; headings are never + translated. +- Boundaries: SPEC-014 (guard command scope) and SPEC-017 (`forge check`, + criterion coverage) do not touch these surfaces; SPEC-015 and SPEC-016 + change contents inside `internal/workflow`, not this mechanism. + +### Tests + +- `internal/workflow/workflow_test.go`: `Meaning` returns a non-empty line + for every state in `All()`; `WaitingFor(Blocked)` starts with + `orchestrator:` and never contains `conductor`. +- `kit/machine_test.go`: `kit.Workflow()` contains `` + and no `| \`proposed\`` table row; every `WaitingFor` role token is + `anyone`, `nobody`, or in `kit.Roles()`, and `kit.Roles()` is + `architect,implementer,orchestrator,reviewer`. +- `internal/cli/machine_test.go`: `forge workflow` lists `workflow.All()` in + order with each state's `Meaning` and `WaitingFor`, and two runs are + byte-identical. `internal/cli/machine_internal_test.go` covers + `renderWorkflow` returning an error when the marker is absent. +- `internal/cli/cli_test.go`: scanning `AGENTS.md`, `kit/AGENTS.md`, + `README.md` and `docs/*.md` finds no `state → state` chain and no + `| \`state\` |` row; `docs/workflow.md` points at `forge workflow`. No + `conductor` in `docs/*.md`, `AGENTS.md`, `kit/AGENTS.md`, + `kit/machine/WORKFLOW.md` or `forge roles` output (the `.forge/` records + are excluded). After `new`/`accept`/`start --by ana`, the spec carries + `orchestrator:` and no `conductor:`, and `forge status SPEC-001` shows + `orchestrator ana`; the spec folder holds only `spec.md` (`plan.md` and + `tasks.md` are absent), and `docs/cli.md` no longer claims `forge start` + creates them. A real proposal under `## Convenciones propuestas` does not + block `forge archive`, while one under `## Proposed conventions` does. +- `internal/project/project_test.go`: a spec with legacy `conductor: ana` + loads `Orchestrator == "ana"` and the next `Save` writes `orchestrator:` + and deletes `conductor:`; `Criteria`, `Contract` and `OpenQuestions` are + empty for a body with only `## Criterios de aceptación`, `## Contrato` and + `## Preguntas abiertas`. +- `internal/view/view_test.go`: `Detail` prints `orchestrator ana` for a + spec with an orchestrator. +- `docs`: a test reads `docs/customizing.md` and fails if it still claims the + parser accepts `## Criterios de aceptación`. + +### Out of scope of this contract + +- Renaming any state, or adding or removing a transition (SPEC-015, + SPEC-016). +- The fast lane for bug fixes and chores (SPEC-016). +- `forge check` and criterion-to-task/evidence coverage (SPEC-017). +- The guard's command scoping (SPEC-014). +- Rewriting delivered `.forge/specs/*` history or `.forge/decisions/*` + records to say `orchestrator`. +- A migration command for the `conductor` key; the read fallback plus the + next-save rewrite is the whole mechanism. +- Translating section headings through `working_language`. ## Out of scope -A closed list. +- The contents of the workflow: no state, transition, role or gate is added + or removed here; this spec only unifies where they are declared and how + they are rendered. +- The fast lane (SPEC-016), the criterion checks (SPEC-017) and the guard + scoping (SPEC-014). +- `.forge/` records — delivered specs, decisions and conventions — are + history and are not rewritten. ## History Written by `forge`. Do not edit by hand. +- 2026-09-20 accepted by TheJisus28 +- 2026-09-20 specifying by TheJisus28 +- 2026-09-20 awaiting-approval by orchestrator +- 2026-09-20 planning by TheJisus28 +- 2026-09-20 implementing by orchestrator +- 2026-09-20 reviewing by orchestrator +- 2026-09-20 done by orchestrator: archived diff --git a/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/tasks.md b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/tasks.md new file mode 100644 index 0000000..8e70049 --- /dev/null +++ b/.forge/specs/SPEC-018-keep-one-source-of-truth-for-the-workflow-and/tasks.md @@ -0,0 +1,86 @@ +# Tasks — SPEC-018 + +The phases from the plan, as checkboxes. One phase is one implementer run +and one commit. Tick a phase when it lands and say where the work is, so a +later spec knows what exists without reading the diff. + +- [x] Phase 1 — Workflow states: one declaration, rendered. Where: + `internal/workflow/workflow.go` (`Meaning`, `WaitingFor`), + `kit/machine/WORKFLOW.md` (marker), + `internal/cli/machine.go` (`cmdWorkflow`, `renderWorkflow`), tests in + `internal/workflow/workflow_test.go`, `internal/cli/machine_test.go`, + `internal/cli/machine_internal_test.go`, `kit/machine_test.go`. + Landed: `workflow.Meaning` covers every `All()` state; `WaitingFor(Blocked)` + is `orchestrator: clear the blocker`; `WORKFLOW.md` holds + `` after `## States`; `renderWorkflow` replaces the + marker with a table built from `All()`/`Meaning()`/`WaitingFor()` and errors + naming `kit/machine/WORKFLOW.md` when the marker is absent. Verified with + `go test ./...` (all packages ok), `gofmt -l .` (no output), `go vet ./...` + (no output) and `go run . workflow` (table generated in lifecycle order). +- [x] Phase 2 — Pages stop restating; `docs/cli` matches behaviour. Where: + `docs/workflow.md`, `AGENTS.md`, `kit/AGENTS.md`, `README.md`, + `docs/cli.md`, tests in `internal/cli/cli_test.go`. + Landed: the loop diagram and `## States` table are gone from + `docs/workflow.md`, replaced by a pointer at `forge workflow`; the loop block + is gone from `AGENTS.md`, `kit/AGENTS.md` and `README.md`, each pointing at + `forge workflow`; `docs/cli.md`'s `forge start` paragraph now says start + creates the folder and records fingerprints, not `plan.md`/`tasks.md`, which + planning writes after `forge approve`. + `TestDocs_DoNotRestateTheStateMachine` scans `AGENTS.md`, `kit/AGENTS.md`, + `README.md` and `docs/*.md` for state arrow chains and state table rows + built from `workflow.All()`; `TestDocs_WorkflowPointsAtForgeWorkflow` and + `TestDocs_ForgeStartDoesNotCreatePlanningFiles` pin the pointer and the + corrected command. Verified with `go test ./...` (all packages ok), + `gofmt -l .` (no output) and `go vet ./...` (no output); the three new tests + pass with `-v`, and a standalone reproduction confirmed the patterns match + the pre-change pages and ignore the Conventional Commits arrow in + `AGENTS.md`. +- [x] Phase 3 — One name for the driver: `orchestrator`. Where: + `internal/project/project.go` (`Conductor` → `Orchestrator`, `FromDoc`, + `Save`), `internal/cli/work.go` (`cmdStart`), `internal/view/view.go`, + `docs/teams.md`, `docs/cli.md`, `AGENTS.md`; tests in + `internal/project/project_test.go`, `internal/view/view_test.go`, + `internal/cli/cli_test.go`. + Landed: `project.Spec.Orchestrator` replaces `Conductor`; `orchestratorFrom` + reads `orchestrator` then falls back to the read-only legacy `conductor`, + and `Save` writes `orchestrator` with `setOrDelete` and always deletes + `conductor`. `cmdStart` assigns `s.Orchestrator` and its `--by` help says + "who orchestrates this spec"; `view.Brief` and `view.Detail` use the field + and print `orchestrator`. `docs/teams.md`, `docs/cli.md` (the `forge start` + paragraph now records the orchestrator) and the release step in `AGENTS.md` + say orchestrator; `.forge/` records are untouched and still read through the + fallback (`go run . status SPEC-018` prints `orchestrator TheJisus28` from + the on-disk `conductor:`). + Tests added: `TestSave_MigratesLegacyConductorKey` (legacy loads, next save + rewrites and drops the key), `TestDetail_ShowsOrchestrator`, + `TestStart_RecordsOneOrchestratorName` (after `new`/`accept`/`start --by + ana` the spec has `orchestrator: ana`, no `conductor:`, and `forge status` + shows it), `TestDocs_DoNotSayConductor` (scans `AGENTS.md`, + `kit/AGENTS.md`, `kit/machine/WORKFLOW.md`, `docs/*.md` and `forge roles` + output). Verified with `go test ./...` (all packages ok), `gofmt -l .` (no + output), `go vet ./...` (no output) and `go run . status SPEC-018`. +- [x] Phase 4 — English section headings only. Where: + `internal/project/project.go` (`Criteria`, `Contract`, `OpenQuestions`), + `internal/cli/work.go` (`pendingConventions`), `docs/customizing.md`; + tests in `internal/project/project_test.go`, `internal/cli/cli_test.go`. + Landed: `Criteria`, `Contract` and `OpenQuestions` read only `Acceptance + criteria`, `Contract` and `Open questions`; `pendingConventions` reads only + `Proposed conventions`; the `working_language` paragraph in + `docs/customizing.md` now says the five section headings are fixed English + and `working_language` governs the prose, with no claim that a translated + heading is read. `isNone`'s `ninguna` value is untouched (content, not a + heading; SPEC-019 decision 2). Tests added: + `TestSpec_ReadsEnglishSectionHeadingsOnly` (a body with only `Criterios de + aceptación`, `Contrato` and `Preguntas abiertas` yields empty criteria, + contract and questions), `TestArchive_ReadsOnlyEnglishConventionHeading` + (`Proposed conventions` blocks archive and names the proposal; `Convenciones + propuestas` is invisible and archive succeeds) and + `TestDocs_CustomizingDoesNotAcceptTranslatedHeadings` (names the five fixed + English headings and fails on an acceptance claim for the Spanish alias, + tolerating a negated mention). Verified with `go test ./...` (all packages + ok), `gofmt -l .` (no output) and `go vet ./...` (no output); the three new + tests pass with `-v`. + +## Proposed conventions + +None. diff --git a/AGENTS.md b/AGENTS.md index 2282dd3..abfd423 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,10 +18,8 @@ lives in `.forge/`. ## The loop -``` -proposed → accepted → specifying → awaiting-approval → planning → -implementing → reviewing → done -``` +The states and the legal transitions between them live in the binary: run +`forge workflow` to print them. What matters when working: - Anyone proposes with `forge new`, accepts with `forge accept` and approves a contract with `forge approve`. Forge has no authorization @@ -51,7 +49,7 @@ go run . init /tmp/scratch && go run . status Each merge to `main` that closes a spec is packaged as a release, cut by hand (decision 0002). After the spec's pull request is merged: -1. The conductor proposes the next `vX.Y.Z` from Conventional Commits +1. The orchestrator proposes the next `vX.Y.Z` from Conventional Commits (`fix` → patch, `feat` → minor, breaking → major); the maintainer approves the exact number. 2. Tag the approved commit and push the tag: `git tag vX.Y.Z` then diff --git a/CHANGELOG.md b/CHANGELOG.md index 22f4d07..4721758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- The workflow states, transitions and roles have one source of truth: the + states live in `internal/workflow` and `forge workflow` renders them into + `kit/machine/WORKFLOW.md`, so the docs and the pages no longer restate the + loop and point at the command instead. +- Whoever drives a spec is named `orchestrator` everywhere: the frontmatter + key, `forge status`, `forge roles`, the help text and the docs. Specs + written before the rename keep their recorded actor through a read-only + `conductor` fallback, and `Save` writes only the new key. +- The CLI reads English section headings only (`Acceptance criteria`, + `Contract`, `Open questions`, `Proposed conventions`, `Existing state`); + `working_language` governs the prose, not the headings. `docs/cli.md` no + longer claims `forge start` writes `plan.md` or `tasks.md`. + ## [0.4.0] - 2026-09-20 ### Changed diff --git a/README.md b/README.md index 7b2144e..fd8d0ef 100644 --- a/README.md +++ b/README.md @@ -70,18 +70,14 @@ forge validate # exit 1 when the project is inconsistent ## How the loop works -``` -proposed → accepted → specifying → awaiting-approval → planning → -implementing → reviewing → done -``` - -The spec is born as a problem and a few acceptance criteria, and grows a -contract as it advances. Two moves are worth a human's attention: accepting -work into the queue, and approving the contract. Forge has no maintainer list -to check; anyone can do either, the way anyone with push access can commit. -The record says who did it, and the scrutiny happens in the pull request -review your team already does. [The workflow](docs/workflow.md) has the -detail. +The states and the legal transitions between them live in the binary: +`forge workflow` prints them. The spec is born as a problem and a few +acceptance criteria, and grows a contract as it advances. Two moves are +worth a human's attention: accepting work into the queue, and approving the +contract. Forge has no maintainer list to check; anyone can do either, the +way anyone with push access can commit. The record says who did it, and the +scrutiny happens in the pull request review your team already does. [The +workflow](docs/workflow.md) has the detail. ## What `forge init` plants diff --git a/docs/cli.md b/docs/cli.md index 8a353b4..0e27401 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -58,9 +58,10 @@ this; Forge has no list to check the handle against. Begins the work. Refuses if the spec is not `accepted`, has children, or has open dependencies, and prints what is ready instead. `--force` records the -exception in the spec. Records the contract fingerprints of any -`@contract` dependencies, creates the spec folder's `plan.md` and `tasks.md`, -and prints the branch to create. +exception in the spec. Creates the spec folder, records the orchestrator and +the contract fingerprints of any `@contract` dependencies, and prints the +branch to create. It does not create `plan.md` or `tasks.md`; those are +written during planning, after `forge approve`. ### `forge approve [--by ] [--note ...]` diff --git a/docs/customizing.md b/docs/customizing.md index 1ccbfbf..bd1bd87 100644 --- a/docs/customizing.md +++ b/docs/customizing.md @@ -50,10 +50,11 @@ allowlist is a feature request, not part of this change. ## Working language -`working_language` in `.forge/project.md` applies to specs, decisions and -product copy. The process files stay English so the same kit works across -teams; the parser accepts both `## Acceptance criteria` and -`## Criterios de aceptación` for the sections it needs to read. +`working_language` in `.forge/project.md` applies to the prose inside a +spec, a decision or a convention. The process files and the section headings +stay English so the same kit works across teams: the CLI reads `Acceptance +criteria`, `Contract`, `Open questions`, `Proposed conventions` and +`Existing state`, and a translated heading is not a heading it knows. ## Other agents diff --git a/docs/teams.md b/docs/teams.md index 5ba5501..6df5c0d 100644 --- a/docs/teams.md +++ b/docs/teams.md @@ -85,7 +85,7 @@ deleted, so a later spec can read what was built and why. ## Roles -There is one role that matters to Forge: the **conductor**, whoever takes +There is one role that matters to Forge: the **orchestrator**, whoever takes a spec and drives the agents through it. Anyone can propose a spec, anyone can accept one, anyone can approve a contract. Real teams already know who should weigh in on what; Forge records who did, and does not referee it. diff --git a/docs/workflow.md b/docs/workflow.md index b1c85d8..93f56c9 100644 --- a/docs/workflow.md +++ b/docs/workflow.md @@ -2,32 +2,15 @@ Forge has one artifact: the **spec**. It is born cheap and grows. -``` -proposed → accepted → specifying → awaiting-approval → planning → -implementing → reviewing → done - -any of them → dropped implementing ↔ blocked -``` +The states and the legal transitions between them live in the binary: +`forge workflow` prints them in lifecycle order, with what each state means +and who has to act next. This page explains the process around them and +keeps no second copy of the machine. The backlog is not a folder: it is the specs in `proposed` and `accepted`. An epic is not a type: it is a spec that has children. Collapsing those into one artifact removes the copying, and the drift that copying causes. -## States - -| State | What it means | Who moves it | -|---|---|---| -| `proposed` | Written, not in the queue | anyone, with `forge accept` or drop | -| `accepted` | In the queue | anyone, with `forge start` | -| `specifying` | The contract is being written | architect | -| `awaiting-approval` | Contract ready for a look | anyone, with `forge approve` | -| `planning` | Being split into phases | orchestrator | -| `implementing` | Product code, phase by phase | implementer | -| `blocked` | Cannot continue | conductor | -| `reviewing` | Criteria being verified | reviewer | -| `done` | Archived | nobody | -| `dropped` | Will not be done | nobody | - Only `forge` writes `status`, and every move appends a line to the spec's History section saying who did it and why. diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index e55c13d..38ea9ac 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -5,10 +5,12 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strings" "testing" "github.com/TheJisus28/forge/internal/cli" + "github.com/TheJisus28/forge/internal/workflow" ) // runGit runs a git command in dir and fails the test when it fails. @@ -87,6 +89,27 @@ func read(t *testing.T, path string) string { return string(data) } +// docsSection returns the text under the heading whose line starts with +// heading, up to the next heading. It scopes a docs check to the command it +// documents instead of the whole page. +func docsSection(body, heading string) string { + var b strings.Builder + found := false + for _, line := range strings.SplitAfter(body, "\n") { + if !found { + if strings.HasPrefix(line, heading) { + found = true + } + continue + } + if strings.HasPrefix(line, "##") { + break + } + b.WriteString(line) + } + return b.String() +} + // doneSpec writes a delivered spec straight to disk: the capability view only // reads the file, so a test needs no full lifecycle to have something done. func doneSpec(t *testing.T, dir, id, title, capability string, supersedes ...string) string { @@ -319,7 +342,7 @@ func TestLifecycle(t *testing.T) { body = read(t, path) write(t, path, strings.Replace(body, "## Contract\n", "## Contract\n\nGET /cards\n", 1)) mustRun(t, dir, "advance", "SPEC-001", "--to", "awaiting-approval", "--by", "ana") - // The conductor approving their own contract is fine: there is no + // The orchestrator approving their own contract is fine: there is no // separate approver role to ask, and the record still says it was ana. mustRun(t, dir, "approve", "SPEC-001", "--by", "ana") if !strings.Contains(read(t, path), "contract_hash:") { @@ -393,6 +416,44 @@ func TestArchive_IgnoresTemplateConventionsComment(t *testing.T) { } } +// The CLI reads English section headings only: a proposal under the retired +// Spanish heading is invisible, while one under `Proposed conventions` blocks +// archiving (AC4, SPEC-018 decision 6). +func TestArchive_ReadsOnlyEnglishConventionHeading(t *testing.T) { + prepare := func(t *testing.T) (string, string) { + t.Helper() + dir := newRepo(t) + specDir := filepath.Join(dir, ".forge", "specs", "SPEC-001-a-change") + write(t, filepath.Join(specDir, "spec.md"), + "---\nid: SPEC-001\ntitle: A change\nstatus: reviewing\ncapability: workflow\n---\n\n## Contract\n\nx\n") + write(t, filepath.Join(specDir, "review.md"), "Verdict: pass\n") + return dir, specDir + } + + t.Run("English heading blocks", func(t *testing.T) { + dir, specDir := prepare(t) + write(t, filepath.Join(specDir, "tasks.md"), + "# Tasks\n\n- [x] Phase 1\n\n## Proposed conventions\n\nErrors use an envelope.\n") + out, code := run(t, dir, "archive", "SPEC-001") + if code == 0 { + t.Fatalf("a proposal under the English heading should block archiving:\n%s", out) + } + if !strings.Contains(out, "Errors use an envelope.") { + t.Errorf("the proposal should be the reported line:\n%s", out) + } + }) + + t.Run("Spanish heading is invisible", func(t *testing.T) { + dir, specDir := prepare(t) + write(t, filepath.Join(specDir, "tasks.md"), + "# Tasks\n\n- [x] Phase 1\n\n## Convenciones propuestas\n\nErrors use an envelope.\n") + mustRun(t, dir, "archive", "SPEC-001") + if body := read(t, filepath.Join(specDir, "spec.md")); !strings.Contains(body, "status: done") { + t.Errorf("a proposal under the Spanish heading should not block archiving:\n%s", body) + } + }) +} + func TestHierarchyAndDependencies(t *testing.T) { dir := newRepo(t) specs := filepath.Join(dir, ".forge", "specs") @@ -827,6 +888,140 @@ func TestDocs_DescribeCapabilities(t *testing.T) { } } +// The pages an agent or a person reads must not keep a second copy of the +// state machine: an arrow chain between two states, or a table row whose first +// cell is a state. The names come from the binary, so a page cannot keep a +// list that diverged from it (AC1, AC5). +func TestDocs_DoNotRestateTheStateMachine(t *testing.T) { + pages := []string{"../../AGENTS.md", "../../kit/AGENTS.md", "../../README.md"} + docs, err := filepath.Glob("../../docs/*.md") + if err != nil { + t.Fatal(err) + } + pages = append(pages, docs...) + + names := make([]string, 0, len(workflow.All())) + for _, s := range workflow.All() { + names = append(names, regexp.QuoteMeta(string(s))) + } + alt := strings.Join(names, "|") + chain := regexp.MustCompile(`\b(?:` + alt + `)\s*(?:→|↔)\s*(?:` + alt + `)\b`) + row := regexp.MustCompile("(?m)^\\|\\s*`(?:" + alt + ")`\\s*\\|") + + for _, page := range pages { + body := read(t, page) + if m := chain.FindString(body); m != "" { + t.Errorf("%s restates the state machine as an arrow chain: %q", page, m) + } + if m := row.FindString(body); m != "" { + t.Errorf("%s restates the state machine as a table row: %q", page, m) + } + } +} + +// docs/ explains the process and points at the binary for the machine instead +// of holding an ordered state list (AC5). +func TestDocs_WorkflowPointsAtForgeWorkflow(t *testing.T) { + if !strings.Contains(read(t, "../../docs/workflow.md"), "forge workflow") { + t.Error("docs/workflow.md should point at `forge workflow` for the states and transitions") + } +} + +// `forge start` creates the spec folder and records fingerprints; planning +// writes `plan.md` and `tasks.md` after approval. The page must match the +// command and not claim start creates them (AC3). +func TestDocs_ForgeStartDoesNotCreatePlanningFiles(t *testing.T) { + start := docsSection(read(t, "../../docs/cli.md"), "### `forge start") + if start == "" { + t.Fatal("docs/cli.md should document `forge start`") + } + + claim := regexp.MustCompile("(?i)creates?[^.\\n]{0,60}`(plan\\.md|tasks\\.md)`") + for _, m := range claim.FindAllStringIndex(start, -1) { + if strings.HasSuffix(strings.ToLower(start[:m[0]]), "not ") { + continue + } + t.Errorf("docs/cli.md claims `forge start` creates a planning file: %q", start[m[0]:m[1]]) + } +} + +// The driver is recorded under one key: `forge start` writes `orchestrator` +// and never the retired `conductor`, and `forge status` names it (AC2). +func TestStart_RecordsOneOrchestratorName(t *testing.T) { + dir := newRepo(t) + mustRun(t, dir, "new", "A change", "--capability", "workflow") + path := filepath.Join(dir, ".forge", "specs", "SPEC-001-a-change", "spec.md") + + mustRun(t, dir, "accept", "SPEC-001", "--by", "jesus") + mustRun(t, dir, "start", "SPEC-001", "--by", "ana") + + body := read(t, path) + if !strings.Contains(body, "orchestrator: ana") { + t.Errorf("start should record the orchestrator:\n%s", body) + } + if strings.Contains(body, "conductor:") { + t.Errorf("start should not write the legacy conductor key:\n%s", body) + } + if out := mustRun(t, dir, "status", "SPEC-001"); !strings.Contains(out, "orchestrator ana") { + t.Errorf("status should name the orchestrator:\n%s", out) + } +} + +// One name for the driver in every page a reader or an agent opens, and in the +// roles the binary lists. The `.forge/` records are history and are not +// scanned (AC2). +func TestDocs_DoNotSayConductor(t *testing.T) { + pages := []string{ + "../../AGENTS.md", + "../../kit/AGENTS.md", + "../../kit/machine/WORKFLOW.md", + } + docs, err := filepath.Glob("../../docs/*.md") + if err != nil { + t.Fatal(err) + } + pages = append(pages, docs...) + for _, page := range pages { + if strings.Contains(read(t, page), "conductor") { + t.Errorf("%s should say orchestrator, not conductor", page) + } + } + + if out := mustRun(t, t.TempDir(), "roles"); strings.Contains(out, "conductor") { + t.Errorf("forge roles should not list a conductor:\n%s", out) + } +} + +// Headings are fixed English; `working_language` governs the prose inside a +// spec, decision or convention, never the headings the CLI reads. The page +// names the English headings and makes no claim that a translated heading is +// accepted (AC4, SPEC-018 decision 6). +func TestDocs_CustomizingDoesNotAcceptTranslatedHeadings(t *testing.T) { + doc := strings.Join(strings.Fields(read(t, "../../docs/customizing.md")), " ") + + alias := regexp.QuoteMeta("Criterios de aceptación") + claim := regexp.MustCompile(`(?i)accept(?:s|ed)?[^.;]{0,120}` + alias + + `|` + alias + `[^.;]{0,120}accept(?:s|ed)?`) + for _, m := range claim.FindAllString(doc, -1) { + lower := strings.ToLower(m) + if strings.Contains(lower, "not accept") || + strings.Contains(lower, "no longer accept") || + strings.Contains(lower, "never accept") { + continue + } + t.Errorf("docs/customizing.md claims the parser accepts a translated heading: %q", m) + } + + for _, want := range []string{ + "Acceptance criteria", "Contract", "Open questions", + "Proposed conventions", "Existing state", + } { + if !strings.Contains(doc, want) { + t.Errorf("docs/customizing.md should name the fixed English heading %q", want) + } + } +} + func TestUnknownCommandAndMissingProject(t *testing.T) { dir := t.TempDir() if out, code := run(t, dir, "frobnicate"); code == 0 || !strings.Contains(out, "unknown command") { diff --git a/internal/cli/machine.go b/internal/cli/machine.go index 7d74775..a30889c 100644 --- a/internal/cli/machine.go +++ b/internal/cli/machine.go @@ -1,15 +1,26 @@ package cli import ( + "bytes" "fmt" "io" "strings" + "github.com/TheJisus28/forge/internal/workflow" "github.com/TheJisus28/forge/kit" ) +// workflowFile names the embedded copy for errors, and workflowMarker is the +// placeholder it carries where the states table is rendered from Go. +const ( + workflowFile = "kit/machine/WORKFLOW.md" + workflowMarker = "" +) + // cmdWorkflow prints the process description from the binary. It never reads -// the project, so any agent, wrapped or not, can pull the same copy. +// the project, so any agent, wrapped or not, can pull the same copy. The +// states table is rendered from internal/workflow, which is the only +// declaration of the states. func cmdWorkflow(args []string, out io.Writer) error { fs := newFlagSet("workflow", "usage: forge workflow", out) if _, err := parseArgs(fs, args); err != nil { @@ -19,10 +30,30 @@ func cmdWorkflow(args []string, out io.Writer) error { if err != nil { return err } - _, err = out.Write(data) + rendered, err := renderWorkflow(data) + if err != nil { + return err + } + _, err = out.Write(rendered) return err } +// renderWorkflow replaces the states marker in the embedded workflow with a +// table built from workflow.All(), workflow.Meaning() and +// workflow.WaitingFor(). The Markdown stays prose; the table has one source. +// A workflow that lost the marker is an error naming the file, so the states +// cannot silently disappear from `forge workflow`. +func renderWorkflow(raw []byte) ([]byte, error) { + if !bytes.Contains(raw, []byte(workflowMarker)) { + return nil, fmt.Errorf("%s is missing the %s marker", workflowFile, workflowMarker) + } + rows := []string{"| State | Meaning | Who acts next |", "|---|---|---|"} + for _, s := range workflow.All() { + rows = append(rows, fmt.Sprintf("| `%s` | %s | %s |", s, workflow.Meaning(s), workflow.WaitingFor(s))) + } + return bytes.ReplaceAll(raw, []byte(workflowMarker), []byte(strings.Join(rows, "\n"))), nil +} + // cmdRoles prints the role names, or one role's instructions by name. func cmdRoles(args []string, out io.Writer) error { fs := newFlagSet("roles", "usage: forge roles [name]", out) diff --git a/internal/cli/machine_internal_test.go b/internal/cli/machine_internal_test.go new file mode 100644 index 0000000..11ab878 --- /dev/null +++ b/internal/cli/machine_internal_test.go @@ -0,0 +1,18 @@ +package cli + +import ( + "strings" + "testing" +) + +// A workflow that lost the marker must fail loudly instead of printing a +// page whose states table silently vanished. +func TestRenderWorkflow_MissingMarkerIsAnError(t *testing.T) { + _, err := renderWorkflow([]byte("# Workflow\n\n## States\n\nno marker here\n")) + if err == nil { + t.Fatal("a workflow without the marker should fail") + } + if !strings.Contains(err.Error(), "kit/machine/WORKFLOW.md") { + t.Errorf("the error should name the file, got: %v", err) + } +} diff --git a/internal/cli/machine_test.go b/internal/cli/machine_test.go index 1988c66..eb1d28f 100644 --- a/internal/cli/machine_test.go +++ b/internal/cli/machine_test.go @@ -1,10 +1,13 @@ package cli_test import ( + "fmt" "os" "path/filepath" "strings" "testing" + + "github.com/TheJisus28/forge/internal/workflow" ) // The machinery is served from the binary, so it works in a directory with @@ -17,6 +20,31 @@ func TestWorkflowCommand_PrintsTheWorkflow(t *testing.T) { } } +// The states table is generated from internal/workflow, in lifecycle order, +// and the same bytes are written on every run. +func TestWorkflowCommand_RendersTheStatesFromGo(t *testing.T) { + dir := t.TempDir() + first := mustRun(t, dir, "workflow") + second := mustRun(t, dir, "workflow") + if first != second { + t.Error("two runs of forge workflow are not byte-identical") + } + + last := -1 + for _, s := range workflow.All() { + row := fmt.Sprintf("| `%s` | %s | %s |", s, workflow.Meaning(s), workflow.WaitingFor(s)) + at := strings.Index(first, row) + if at < 0 { + t.Errorf("workflow output missing the %s row %q:\n%s", s, row, first) + continue + } + if at < last { + t.Errorf("state %s is out of lifecycle order:\n%s", s, first) + } + last = at + } +} + func TestRolesCommand_ListsAndPrints(t *testing.T) { dir := t.TempDir() out := mustRun(t, dir, "roles") diff --git a/internal/cli/work.go b/internal/cli/work.go index 1ccafe1..09e6cde 100644 --- a/internal/cli/work.go +++ b/internal/cli/work.go @@ -150,7 +150,7 @@ func cmdAccept(args []string, out io.Writer) error { func cmdStart(args []string, out io.Writer) error { fs := newFlagSet("start", "usage: forge start [--by ] [--force]", out) - by := fs.String("by", "", "who conducts this spec (defaults to git user.name)") + by := fs.String("by", "", "who orchestrates this spec (defaults to git user.name)") force := fs.Bool("force", false, "start despite open dependencies, recording the exception") rest, err := parseArgs(fs, args) if err != nil { @@ -176,11 +176,11 @@ func cmdStart(args []string, out io.Writer) error { return fmt.Errorf("%s", msg) } - conductor, err := resolveActor(p, *by) + orchestrator, err := resolveActor(p, *by) if err != nil { return err } - s.Conductor = conductor + s.Orchestrator = orchestrator for _, d := range s.Deps { if d.Level != "contract" { continue @@ -193,7 +193,7 @@ func cmdStart(args []string, out io.Writer) error { if len(blockers) > 0 { note = "forced despite: " + strings.Join(blockers, "; ") } - s.SetStatus(workflow.Specifying, conductor, note) + s.SetStatus(workflow.Specifying, orchestrator, note) if err := os.MkdirAll(s.Dir(), 0o755); err != nil { return err } @@ -487,10 +487,10 @@ func pendingConventions(dir string) []string { if err != nil { continue } - for _, heading := range []string{"Proposed conventions", "Convenciones propuestas"} { - if body := stripComments(d.Section(heading)); strings.TrimSpace(body) != "" && !isNone(body) { - out = append(out, e.Name()+": "+firstLine(body)) - } + // Headings are fixed English: `working_language` governs the prose, + // never the heading the CLI reads. + if body := stripComments(d.Section("Proposed conventions")); strings.TrimSpace(body) != "" && !isNone(body) { + out = append(out, e.Name()+": "+firstLine(body)) } } return out diff --git a/internal/project/git.go b/internal/project/git.go index a8d5a2e..e697caa 100644 --- a/internal/project/git.go +++ b/internal/project/git.go @@ -77,7 +77,7 @@ func Fetch(root string) error { return cmd.Run() } -// UserName returns the configured git user, used as a default conductor. +// UserName returns the configured git user, used as a default orchestrator. func UserName(root string) string { name, err := run(root, "git", "config", "user.name") if err != nil { diff --git a/internal/project/project.go b/internal/project/project.go index 2f58f90..5aea6ad 100644 --- a/internal/project/project.go +++ b/internal/project/project.go @@ -61,7 +61,7 @@ type Spec struct { External []map[string]string Path string - Conductor string + Orchestrator string AcceptedBy string ApprovedBy string ContractHash string @@ -353,12 +353,11 @@ func (p *Project) Capabilities() []CapabilityGroup { // Criteria parses the acceptance criteria out of the body. var criterionRe = regexp.MustCompile(`^-\s*(?:\[[ xX]\]\s*)?(AC\d+)\s*[:.-]\s*(.+)$`) -// Criteria returns the acceptance criteria declared by the spec. +// Criteria returns the acceptance criteria declared by the spec. Headings are +// fixed English: `working_language` governs the prose, never the heading the +// CLI reads. func (s *Spec) Criteria() []Criterion { body := s.doc.Section("Acceptance criteria") - if body == "" { - body = s.doc.Section("Criterios de aceptación") - } var out []Criterion for _, line := range strings.Split(body, "\n") { if m := criterionRe.FindStringSubmatch(strings.TrimSpace(line)); m != nil { @@ -368,21 +367,17 @@ func (s *Spec) Criteria() []Criterion { return out } -// Contract returns the contract section, empty while it is not written. +// Contract returns the contract section, empty while it is not written. The +// heading is fixed English, never translated. func (s *Spec) Contract() string { - if c := s.doc.Section("Contract"); c != "" { - return c - } - return s.doc.Section("Contrato") + return s.doc.Section("Contract") } // OpenQuestions returns the section that must be settled before the contract -// is approved, empty when it is not written. +// is approved, empty when it is not written. The heading is fixed English, +// never translated. func (s *Spec) OpenQuestions() string { - if q := s.doc.Section("Open questions"); q != "" { - return q - } - return s.doc.Section("Preguntas abiertas") + return s.doc.Section("Open questions") } // HasOpenQuestions reports whether the spec still lists an unanswered @@ -457,7 +452,8 @@ func (s *Spec) Save() error { } else { s.doc.SetMapList("blocked_by_external", s.External) } - setOrDelete(s.doc, "conductor", s.Conductor) + setOrDelete(s.doc, "orchestrator", s.Orchestrator) + s.doc.Delete("conductor") setOrDelete(s.doc, "accepted_by", s.AcceptedBy) setOrDelete(s.doc, "approved_by", s.ApprovedBy) setOrDelete(s.doc, "contract_hash", s.ContractHash) @@ -502,7 +498,7 @@ func FromDoc(path string, d *doc.Doc) (*Spec, error) { Needs: d.List("needs"), External: d.MapList("blocked_by_external"), Path: path, - Conductor: d.Str("conductor"), + Orchestrator: orchestratorFrom(d), AcceptedBy: d.Str("accepted_by"), ApprovedBy: d.Str("approved_by"), ContractHash: d.Str("contract_hash"), @@ -523,6 +519,15 @@ func FromDoc(path string, d *doc.Doc) (*Spec, error) { return s, nil } +// orchestratorFrom reads the driver of a spec. Specs written before the key was +// renamed still carry `conductor`; it is read here and never written again. +func orchestratorFrom(d *doc.Doc) string { + if o := d.Str("orchestrator"); o != "" { + return o + } + return d.Str("conductor") +} + var idRe = regexp.MustCompile(`^SPEC-(\d+)$`) // NormalizeID accepts "4", "spec-4" or "SPEC-004" and returns "SPEC-004". diff --git a/internal/project/project_test.go b/internal/project/project_test.go index 33f4a25..02a8b46 100644 --- a/internal/project/project_test.go +++ b/internal/project/project_test.go @@ -286,7 +286,7 @@ func TestSaveRoundTrip(t *testing.T) { p := load(t) s, _ := p.Spec("SPEC-003") s.SetStatus(workflow.Specifying, "ana", "started") - s.Conductor = "ana" + s.Orchestrator = "ana" s.Capability = "notifications" s.Supersedes = []string{"SPEC-002"} s.Agreed["SPEC-002"] = "abc123" @@ -298,7 +298,7 @@ func TestSaveRoundTrip(t *testing.T) { t.Fatal(err) } reloaded, _ := again.Spec("SPEC-003") - if reloaded.Status != workflow.Specifying || reloaded.Conductor != "ana" { + if reloaded.Status != workflow.Specifying || reloaded.Orchestrator != "ana" { t.Errorf("not persisted: %+v", reloaded) } if reloaded.Capability != "notifications" { @@ -332,6 +332,72 @@ func TestSaveRoundTrip(t *testing.T) { } } +// A spec written before the key was renamed still reads its driver from the +// legacy `conductor`, and the next save writes the single `orchestrator` key +// and drops the old one (SPEC-018, decision 4). +func TestSave_MigratesLegacyConductorKey(t *testing.T) { + root := write(t, config, map[string]string{ + "SPEC-001-legacy.md": "---\nid: SPEC-001\ntitle: Legacy\nstatus: planning\n" + + "capability: workflow\nconductor: ana\n---\n\n## Contract\n\nx\n", + }) + p, err := project.Load(root) + if err != nil { + t.Fatal(err) + } + s, ok := p.Spec("SPEC-001") + if !ok { + t.Fatal("SPEC-001 should load") + } + if s.Orchestrator != "ana" { + t.Fatalf("legacy conductor should load as the orchestrator: %q", s.Orchestrator) + } + if err := s.Save(); err != nil { + t.Fatal(err) + } + data, err := os.ReadFile(s.Path) + if err != nil { + t.Fatal(err) + } + body := string(data) + if !strings.Contains(body, "orchestrator: ana") { + t.Errorf("the save should write orchestrator: ana:\n%s", body) + } + if strings.Contains(body, "conductor:") { + t.Errorf("the save should delete the legacy conductor key:\n%s", body) + } +} + +// The CLI reads English section headings only. A body that uses the retired +// Spanish aliases yields no criteria, contract or questions, so a second +// language cannot become a second set of headings to read (AC4, SPEC-018 +// decision 6). +func TestSpec_ReadsEnglishSectionHeadingsOnly(t *testing.T) { + root := write(t, config, map[string]string{ + "SPEC-001-spanish.md": "---\nid: SPEC-001\ntitle: Spanish\nstatus: accepted\n" + + "capability: workflow\n---\n\n" + + "## Criterios de aceptación\n\n- AC1: something\n\n" + + "## Contrato\n\nGET /something\n\n" + + "## Preguntas abiertas\n\n- OQ1: something?\n", + }) + p, err := project.Load(root) + if err != nil { + t.Fatal(err) + } + s, ok := p.Spec("SPEC-001") + if !ok { + t.Fatal("SPEC-001 should load") + } + if got := s.Criteria(); len(got) != 0 { + t.Errorf("a Spanish-only body should yield no criteria: %v", got) + } + if got := s.Contract(); got != "" { + t.Errorf("a Spanish-only body should yield no contract: %q", got) + } + if got := s.OpenQuestions(); got != "" { + t.Errorf("a Spanish-only body should yield no open questions: %q", got) + } +} + func TestValidCapability(t *testing.T) { if !project.ValidCapability("guard") { t.Error("guard is a valid capability") diff --git a/internal/validate/validate_test.go b/internal/validate/validate_test.go index e517afe..6cb838d 100644 --- a/internal/validate/validate_test.go +++ b/internal/validate/validate_test.go @@ -327,7 +327,7 @@ func TestRun_AnyHandleCanAcceptOrApprove(t *testing.T) { p := build(t, map[string]string{ "SPEC-001-a.md": "---\nid: SPEC-001\ntitle: A\nstatus: accepted\n" + "accepted_by: pedro\n---\n", - "SPEC-002-b.md": "---\nid: SPEC-002\ntitle: B\nstatus: planning\nconductor: ana\n" + + "SPEC-002-b.md": "---\nid: SPEC-002\ntitle: B\nstatus: planning\norchestrator: ana\n" + "approved_by: ana\ncontract_hash: " + hash + "\n---\n\n## Contract\n\nx\n", }) if got := findings(t, p); len(got) != 0 { diff --git a/internal/view/view.go b/internal/view/view.go index 24578fd..ea4cb78 100644 --- a/internal/view/view.go +++ b/internal/view/view.go @@ -72,8 +72,8 @@ func Brief(p *project.Project) string { return "forge approve" }) section(&b, "in flight", inFlight, func(s *project.Spec) string { - if s.Conductor != "" { - return string(s.Status) + ", " + s.Conductor + if s.Orchestrator != "" { + return string(s.Status) + ", " + s.Orchestrator } return string(s.Status) }) @@ -267,8 +267,8 @@ func Detail(p *project.Project, s *project.Spec) string { if done, total := s.TaskProgress(); total > 0 { fmt.Fprintf(&b, "tasks %d/%d\n", done, total) } - if s.Conductor != "" { - fmt.Fprintf(&b, "conductor %s\n", s.Conductor) + if s.Orchestrator != "" { + fmt.Fprintf(&b, "orchestrator %s\n", s.Orchestrator) } if s.ApprovedBy != "" { fmt.Fprintf(&b, "approved %s, contract %s\n", s.ApprovedBy, s.ContractHash) diff --git a/internal/view/view_test.go b/internal/view/view_test.go index dd07ae5..c9109b3 100644 --- a/internal/view/view_test.go +++ b/internal/view/view_test.go @@ -101,3 +101,15 @@ func TestDetail_ShowsNone(t *testing.T) { t.Errorf("an empty capability should render as (none):\n%s", out) } } + +// The driver is named once, under `orchestrator`, so the detail matches the +// frontmatter key (SPEC-018, decision 4). +func TestDetail_ShowsOrchestrator(t *testing.T) { + p := doneProject(t, 1) + p.Specs[0].Orchestrator = "ana" + + out := view.Detail(p, p.Specs[0]) + if !strings.Contains(out, "orchestrator ana") { + t.Errorf("the detail should show the orchestrator:\n%s", out) + } +} diff --git a/internal/workflow/workflow.go b/internal/workflow/workflow.go index 2d5dbf5..489f910 100644 --- a/internal/workflow/workflow.go +++ b/internal/workflow/workflow.go @@ -32,6 +32,34 @@ func All() []State { Implementing, Blocked, Reviewing, Done, Dropped} } +// Meaning returns the one-line explanation of a state, the normative copy +// `forge workflow` renders into its table. +func Meaning(s State) string { + switch s { + case Proposed: + return "Written, not in the queue" + case Accepted: + return "In the queue" + case Specifying: + return "The contract is being written" + case AwaitingApproval: + return "Contract ready" + case Planning: + return "Splitting into phases" + case Implementing: + return "Product code, phase by phase" + case Blocked: + return "Cannot continue" + case Reviewing: + return "Verifying acceptance criteria" + case Done: + return "Archived and closed" + case Dropped: + return "Will not be done" + } + return "" +} + // Valid reports whether s is a known state. func Valid(s State) bool { for _, k := range All() { @@ -140,7 +168,7 @@ func WaitingFor(s State) string { case Implementing: return "implementer: next phase" case Blocked: - return "conductor: clear the blocker" + return "orchestrator: clear the blocker" case Reviewing: return "reviewer: verify acceptance criteria" case Done: diff --git a/internal/workflow/workflow_test.go b/internal/workflow/workflow_test.go index 9f93a82..7340492 100644 --- a/internal/workflow/workflow_test.go +++ b/internal/workflow/workflow_test.go @@ -60,6 +60,26 @@ func TestCheck_SameState(t *testing.T) { } } +// The table in `forge workflow` has one source, so every state must carry a +// meaning and the blocked line must name a role `forge roles` lists. +func TestMeaning_EveryStateHasALine(t *testing.T) { + for _, s := range workflow.All() { + if strings.TrimSpace(workflow.Meaning(s)) == "" { + t.Errorf("Meaning(%s) is empty", s) + } + } +} + +func TestWaitingFor_BlockedNamesTheOrchestrator(t *testing.T) { + got := workflow.WaitingFor(workflow.Blocked) + if !strings.HasPrefix(got, "orchestrator:") { + t.Errorf("WaitingFor(blocked) = %q, want an orchestrator action", got) + } + if strings.Contains(got, "conductor") { + t.Errorf("WaitingFor(blocked) still says conductor: %q", got) + } +} + func TestInFlightAndTerminal(t *testing.T) { if workflow.InFlight(workflow.Proposed) || workflow.InFlight(workflow.Done) { t.Error("proposed and done are not in flight") diff --git a/kit/AGENTS.md b/kit/AGENTS.md index 8548bfc..c1c47bd 100644 --- a/kit/AGENTS.md +++ b/kit/AGENTS.md @@ -17,10 +17,8 @@ agreement lives in `.forge/`. Keep this file short; it is read every session. ## The loop -``` -proposed → accepted → specifying → awaiting-approval → planning → -implementing → reviewing → done -``` +The states and the legal transitions between them live in the binary: run +`forge workflow` to print them. - Anyone proposes: `forge new ""`. - Anyone accepts (`forge accept`) and approves a contract (`forge diff --git a/kit/machine/WORKFLOW.md b/kit/machine/WORKFLOW.md index 5c164fc..a481b54 100644 --- a/kit/machine/WORKFLOW.md +++ b/kit/machine/WORKFLOW.md @@ -7,18 +7,7 @@ item and no separate epic: the backlog is the specs in `proposed` or ## States -| State | Meaning | Who acts next | -|---|---|---| -| `proposed` | Written, not in the queue | anyone: `forge accept` or drop | -| `accepted` | In the queue | anyone: `forge start` | -| `specifying` | The contract is being written | architect | -| `awaiting-approval` | Contract ready | anyone: `forge approve` | -| `planning` | Splitting into phases | orchestrator | -| `implementing` | Product code, phase by phase | implementer | -| `blocked` | Cannot continue | conductor | -| `reviewing` | Verifying acceptance criteria | reviewer | -| `done` | Archived and closed | nobody | -| `dropped` | Will not be done | nobody | +<!-- forge:states --> Forge has no authorization model. Anyone can accept work into the queue and anyone can approve a contract, the same way anyone with push access can diff --git a/kit/machine_test.go b/kit/machine_test.go index 9e01240..417a3da 100644 --- a/kit/machine_test.go +++ b/kit/machine_test.go @@ -4,6 +4,7 @@ import ( "strings" "testing" + "github.com/TheJisus28/forge/internal/workflow" "github.com/TheJisus28/forge/kit" ) @@ -42,3 +43,42 @@ func TestMachine_ServesWorkflowRolesAndTemplates(t *testing.T) { t.Error("Template(nope) should fail") } } + +// The embedded workflow carries the marker, not a second states table, and +// every "who acts next" line names a role kit.Roles() lists. +func TestWorkflow_StatesAreRenderedFromTheWorkflowPackage(t *testing.T) { + raw, err := kit.Workflow() + if err != nil { + t.Fatalf("Workflow: %v", err) + } + body := string(raw) + if !strings.Contains(body, "<!-- forge:states -->") { + t.Errorf("Workflow() lost the forge:states marker:\n%s", body) + } + if strings.Contains(body, "| `proposed`") { + t.Errorf("Workflow() still carries a hand-written states table:\n%s", body) + } + + roles := kit.Roles() + for _, s := range workflow.All() { + line := workflow.WaitingFor(s) + token, _, ok := strings.Cut(line, ":") + if !ok { + t.Errorf("WaitingFor(%s) has no role token: %q", s, line) + continue + } + if token == "anyone" || token == "nobody" { + continue + } + found := false + for _, r := range roles { + if r == token { + found = true + break + } + } + if !found { + t.Errorf("WaitingFor(%s) names %q, which is not in kit.Roles()", s, token) + } + } +}