feat(software-factory): add @swamp/software-factory extension #58

Merged

adamhjk merged 2 commits from software-factory into main 2026-06-16 21:03:40 +00:00

Conversation 3 Commits 2 Files changed 39 +11496 -2

adamhjk commented 2026-06-16 20:29:15 +00:00

Owner

Copy link

Adds the @swamp/software-factory extension: a generic, model-driven state
machine for guiding agents through a software development lifecycle. Stages,
transitions, gates, required artifacts, review skills, and prompts all live in
the definition's globalArguments as data — the engine ships no lifecycle
concepts (adversarial review, testing, release, UAT are expressible, never
assumed; see examples/).

Ported from the standalone software-factory repo to match the structure of the
existing extensions (model + skill + report, cf. issue-lifecycle):

• Renamed the package scope @adam -> @swamp throughout (manifest, model
  code, report type names, skill docs, examples).
• Pointed manifest.yaml repository at this repo; normalised .swamp.yaml to
  the tool: claude form; adopted the skill-tracking .gitignore pattern.
• Ships the software-factory skill, model engine + _lib, the
  work-item-summary report, and three example factory definitions.

CI

The CI workflow enumerates extensions statically, so this PR also wires
software-factory into .forgejo/workflows/ci.yml, mirroring the cve
pattern (models + reports): change-detection filter, check/lint/fmt/test
matrix, lockfile verification, deps-audit directory, and the new jobs added to
the claude-review, claude-adversarial-review (incl. its trigger condition), and
merge-gate dependency lists. Publishing is already generic (publish.yml
diffs */manifest.yaml), so no change was needed there.

Verification: deno task check / lint / fmt:check clean,
deno install --frozen passes, 143 tests pass / 0 failures. The exact CI
commands were run locally and pass.

🤖 Generated with Claude Code

Adds the `@swamp/software-factory` extension: a generic, model-driven state machine for guiding agents through a software development lifecycle. Stages, transitions, gates, required artifacts, review skills, and prompts all live in the definition's `globalArguments` as data — the engine ships no lifecycle concepts (adversarial review, testing, release, UAT are expressible, never assumed; see `examples/`). Ported from the standalone software-factory repo to match the structure of the existing extensions (model + skill + report, cf. `issue-lifecycle`): - Renamed the package scope `@adam` -> `@swamp` throughout (manifest, model code, report type names, skill docs, examples). - Pointed `manifest.yaml` repository at this repo; normalised `.swamp.yaml` to the `tool: claude` form; adopted the skill-tracking `.gitignore` pattern. - Ships the `software-factory` skill, model engine + `_lib`, the work-item-summary report, and three example factory definitions. ## CI The CI workflow enumerates extensions statically, so this PR also wires `software-factory` into `.forgejo/workflows/ci.yml`, mirroring the `cve` pattern (models + reports): change-detection filter, check/lint/fmt/test matrix, lockfile verification, deps-audit directory, and the new jobs added to the claude-review, claude-adversarial-review (incl. its trigger condition), and merge-gate dependency lists. Publishing is already generic (`publish.yml` diffs `*/manifest.yaml`), so no change was needed there. Verification: `deno task check` / `lint` / `fmt:check` clean, `deno install --frozen` passes, 143 tests pass / 0 failures. The exact CI commands were run locally and pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

adamhjk added 2 commits 2026-06-16 20:29:15 +00:00

feat(software-factory): add @swamp/software-factory extension ... 89eda54d47

A generic, model-driven state machine for guiding agents through a software
development lifecycle. Stages, transitions, gates, required artifacts, review
skills, and prompts all live in the definition's globalArguments as data.

Ported from the standalone software-factory repo to match the structure of
the existing extensions (model + skill + report, cf. issue-lifecycle):
renamed the package scope to @swamp, pointed the manifest at this repo,
normalised .swamp.yaml, and adopted the skill-tracking .gitignore pattern.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ci: run software-factory checks and lockfile verification ... c77fbf5d9a

The CI workflow enumerates extensions statically, so the new
software-factory extension needs its own jobs to be tested. Mirror the cve
pattern (models + reports): add the change-detection filter, check/lint/fmt/
test matrix, lockfile job, and deps-audit directory.

Wire the new jobs into the claude-review, claude-adversarial-review, and
merge-gate dependency lists, and add software-factory to the adversarial
review's trigger condition so changes to this extension get the full review
treatment.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

stack72 commented 2026-06-16 20:40:04 +00:00

Owner

Copy link

CI Security Review

Summary

This PR adds CI check and lockfile verification jobs for the new software-factory extension, following the identical pattern used by all other extensions (cve, kubernetes, workflows, etc.). The changes are:

1. Change detection: Adds software-factory output to the changes job (lines 30, 68)
2. New jobs: software-factory-check and software-factory-lockfile (lines 406–449) — standard deno check/lint/fmt/test matrix
3. Dependency audit: Adds software-factory to the find search paths (line 673)
4. Job dependencies: Adds the new jobs to needs: arrays for claude-review, claude-adversarial-review, and merge-gate
5. Adversarial review trigger: Adds software-factory to the conditional that gates the adversarial review (line 870)

Security Analysis

1. Prompt Injection — No new LLM prompts introduced. Only needs: arrays updated on existing Claude review jobs. ✅

2. Expression Injection — ${{ matrix.task }} is used in run: blocks (lines 425–433), but the matrix values are hardcoded literals ([check, lint, fmt, test]) defined in the workflow YAML itself. Not attacker-controllable. Same pattern as all existing extension jobs. ✅

3. Dangerous Triggers — No trigger changes. Workflow remains pull_request only (not pull_request_target). ✅

4. Supply Chain — New jobs use actions/checkout@v6 and denoland/setup-deno@v2, both trusted publishers acceptable with tag-only pins per policy. No new third-party actions. ✅

5. Permissions — No new permissions. New jobs inherit the workflow-level contents: read. ✅

6. Secret Exposure — New jobs use no secrets. ✅

7. Auto-merge & Trust Boundaries — No changes to merge logic. New jobs correctly added to the merge gate. ✅

Verdict

PASS — Security-neutral change. Boilerplate addition of CI jobs for a new extension, following established patterns with no new attack surface.

## CI Security Review ### Summary This PR adds CI check and lockfile verification jobs for the new `software-factory` extension, following the identical pattern used by all other extensions (cve, kubernetes, workflows, etc.). The changes are: 1. **Change detection**: Adds `software-factory` output to the `changes` job (lines 30, 68) 2. **New jobs**: `software-factory-check` and `software-factory-lockfile` (lines 406–449) — standard deno check/lint/fmt/test matrix 3. **Dependency audit**: Adds `software-factory` to the `find` search paths (line 673) 4. **Job dependencies**: Adds the new jobs to `needs:` arrays for `claude-review`, `claude-adversarial-review`, and `merge-gate` 5. **Adversarial review trigger**: Adds `software-factory` to the conditional that gates the adversarial review (line 870) ### Security Analysis **1. Prompt Injection** — No new LLM prompts introduced. Only `needs:` arrays updated on existing Claude review jobs. ✅ **2. Expression Injection** — `${{ matrix.task }}` is used in `run:` blocks (lines 425–433), but the matrix values are hardcoded literals (`[check, lint, fmt, test]`) defined in the workflow YAML itself. Not attacker-controllable. Same pattern as all existing extension jobs. ✅ **3. Dangerous Triggers** — No trigger changes. Workflow remains `pull_request` only (not `pull_request_target`). ✅ **4. Supply Chain** — New jobs use `actions/checkout@v6` and `denoland/setup-deno@v2`, both trusted publishers acceptable with tag-only pins per policy. No new third-party actions. ✅ **5. Permissions** — No new permissions. New jobs inherit the workflow-level `contents: read`. ✅ **6. Secret Exposure** — New jobs use no secrets. ✅ **7. Auto-merge & Trust Boundaries** — No changes to merge logic. New jobs correctly added to the merge gate. ✅ ### Verdict **PASS** — Security-neutral change. Boilerplate addition of CI jobs for a new extension, following established patterns with no new attack surface.

stack72 commented 2026-06-16 20:42:21 +00:00

Owner

Copy link

Code Review

Blocking Issues

None.

Suggestions

1. mermaid.ts has no test coverage. Every other _lib/ file has a corresponding _test.ts, but mermaid.ts — which provides renderMermaid and renderTables used by the describe method — has none. The rendering logic is deterministic and straightforward to unit-test against a known fixture definition.

2. CI test flags differ from deno.json's test task. The CI workflow runs:

   deno test --allow-read --allow-write --allow-env --allow-net --allow-sys ...
   

   while deno.json defines:

   "test": "deno test --allow-read --allow-write --allow-env ..."
   

   If cel-js (imported live in software_factory_test.ts) requires --allow-sys at runtime, deno task test will fail for those tests locally. Align the flags so the documented local command matches CI.

3. buildCelContext in gates.ts silently coalesces hyphen/underscore name collisions. celName() maps both plan-review and plan_review to the same CEL key plan_review. The graph validator (graph.ts) enforces global uniqueness of artifact names but compares the raw names (which are distinct), so a definition with both would pass validation yet produce a silently shadowed key in every cel gate evaluation. A guard in validateGraph — or at least in buildCelContext — would prevent this footgun.

4. Unescaped interpolation in CEL predicates. Both queryRunDataReader.versionsOf in summary.ts and workflowSucceeded in gates.ts build CEL predicate strings via template literals:

   `modelName == "${opts.modelName}" && name == "${name}" && ...`
   `workflowRunId == "${summary.workflowRunId}" && name == "${physical}" ...`
   

   In practice the values are swamp-managed (NameSchema-validated slugs, platform-generated workflow run IDs), so the risk is low. However, a workflow run ID containing an unescaped " in a corrupted or adversarially-crafted record would break the predicate. Quoting/escaping the interpolated values, or validating them first, would be more defensive.

## Code Review ### Blocking Issues None. ### Suggestions 1. **`mermaid.ts` has no test coverage.** Every other `_lib/` file has a corresponding `_test.ts`, but `mermaid.ts` — which provides `renderMermaid` and `renderTables` used by the `describe` method — has none. The rendering logic is deterministic and straightforward to unit-test against a known fixture definition. 2. **CI test flags differ from `deno.json`'s test task.** The CI workflow runs: ``` deno test --allow-read --allow-write --allow-env --allow-net --allow-sys ... ``` while `deno.json` defines: ``` "test": "deno test --allow-read --allow-write --allow-env ..." ``` If `cel-js` (imported live in `software_factory_test.ts`) requires `--allow-sys` at runtime, `deno task test` will fail for those tests locally. Align the flags so the documented local command matches CI. 3. **`buildCelContext` in `gates.ts` silently coalesces hyphen/underscore name collisions.** `celName()` maps both `plan-review` and `plan_review` to the same CEL key `plan_review`. The graph validator (`graph.ts`) enforces global uniqueness of artifact names but compares the raw names (which are distinct), so a definition with both would pass validation yet produce a silently shadowed key in every `cel` gate evaluation. A guard in `validateGraph` — or at least in `buildCelContext` — would prevent this footgun. 4. **Unescaped interpolation in CEL predicates.** Both `queryRunDataReader.versionsOf` in `summary.ts` and `workflowSucceeded` in `gates.ts` build CEL predicate strings via template literals: ```ts `modelName == "${opts.modelName}" && name == "${name}" && ...` `workflowRunId == "${summary.workflowRunId}" && name == "${physical}" ...` ``` In practice the values are swamp-managed (NameSchema-validated slugs, platform-generated workflow run IDs), so the risk is low. However, a workflow run ID containing an unescaped `"` in a corrupted or adversarially-crafted record would break the predicate. Quoting/escaping the interpolated values, or validating them first, would be more defensive.

stack72 commented 2026-06-16 20:44:29 +00:00

Owner

Copy link

Adversarial Review

Medium

1. gates.ts:347-356 / graph.ts — CEL name collision allows silent artifact/evidence shadowing in gate evaluation.

   celName() (definition_schema.ts:400) maps - to _, so two artifacts with names review-plan and review_plan both become review_plan in the CEL context. In buildCelContext, the second map iteration silently overwrites the first:

   for (const [name, view] of ctx.view.artifacts) {
     artifacts[celName(name)] = view.latest.payload;
   }
   

   Graph validation (graph.ts:75-83) checks for duplicate raw artifact names but not post-celName duplicates. A CEL gate expression like artifacts.review_plan.summary != "" would evaluate against whichever artifact happened to iterate last in the Map — producing silently wrong gate results. The same applies to evidence names (gates.ts:354-356) and approval gate IDs (gates.ts:359).

   Breaking input: A definition with artifacts code-review and code_review on different stages. A CEL gate referencing artifacts.code_review reads the wrong payload.

   Suggested fix: Add a celName-uniqueness check to validateGraph — after collecting all artifact names, also build a Set<string> of celName(name) values and error on collision.

2. artifact_schema.ts:55 — ReDoS via user-controlled regex pattern in artifact schemas.

   if (decl.pattern !== undefined) s = s.regex(new RegExp(decl.pattern));
   

   The pattern field from a factory definition's artifact schema is compiled into a RegExp without any complexity check. A pathological pattern like (a+)+$ causes catastrophic backtracking when matching certain inputs. This runs both at definition validation time (compileArtifactSchema via validateGraph) and at payload recording time (validateArtifactPayload). A crafted payload like "aaa...aab" against such a pattern would block the engine thread.

   Breaking input: An artifact declares schema: { type: "string", pattern: "(a+)+$" }, then record_artifact is called with payload containing "aaaaaaaaaaaaaaaaaaaaaaaaaaab".

   Suggested fix: Either impose a max pattern length or set a timeout/complexity limit. Alternatively, wrap new RegExp(decl.pattern) in a try-catch at compilation time (it already is, in validateGraph), and consider using a regex-safe library or at minimum documenting the operator trust boundary.

Low

3. gates.ts:309 — cooldown gate silently passes on corrupted recordedAt timestamp.

   const elapsed = (ctx.now.getTime() - new Date(recordedAt).getTime()) / 1000;
   if (elapsed < gate.config.seconds) { /* fail */ }
   return pass(gate);
   

   If recordedAt is not a valid ISO date, new Date(recordedAt).getTime() returns NaN, making elapsed NaN. Since NaN < anything is false, the cooldown check is skipped and the gate passes unconditionally. This only matters with corrupted data (the engine always writes new Date().toISOString()), but a cooldown gate is a safety control — failing open is the wrong default.

   Suggested fix: Add if (Number.isNaN(elapsed)) return fail(gate, "invalid timestamp on ...") before the comparison.

4. gates.ts:498-504 — unescaped string interpolation in workflow-succeeded query predicates.

   outputs = await ctx.queryData(
     `workflowRunId == "${summary.workflowRunId}" && name == "${physical}"${selfClause}`,
   );
   

   summary.workflowRunId is parsed from JSON content stored in the data repository. If that content contains a " character (corrupted or crafted data), it breaks the CEL predicate syntax or alters its semantics. Attack surface is limited to compromised platform data — not user-facing input.

   Suggested fix: Escape double-quotes in interpolated values, or use a parameterized query interface if the platform supports one.

5. software_factory.ts:1086-1176 — non-atomic read-modify-write in resolve_findings.

   resolve_findings reads the current artifact, mutates the findings array, and writes back. Two concurrent calls targeting the same artifact would each read the same version, apply their own resolutions, and write — the last write wins, silently dropping the other's resolutions. The model code does not use optimistic concurrency (e.g., compare-and-swap on version).

   Suggested fix: Either version-check before writing (fail if the version changed since read) or document that concurrent resolve_findings calls on the same artifact are unsupported.

Verdict

PASS — The code is well-structured with thorough validation, defense-in-depth gate re-evaluation, and good error messages. The findings above are edge cases: the CEL name collision (#1) and ReDoS (#2) are the most worth addressing before this ships, but neither is exploitable from the normal driver/user path since definitions are operator-authored. No critical or high severity issues found.

## Adversarial Review ### Medium 1. **`gates.ts:347-356` / `graph.ts` — CEL name collision allows silent artifact/evidence shadowing in gate evaluation.** `celName()` (`definition_schema.ts:400`) maps `-` to `_`, so two artifacts with names `review-plan` and `review_plan` both become `review_plan` in the CEL context. In `buildCelContext`, the second map iteration silently overwrites the first: ```typescript for (const [name, view] of ctx.view.artifacts) { artifacts[celName(name)] = view.latest.payload; } ``` Graph validation (`graph.ts:75-83`) checks for duplicate raw artifact names but not post-`celName` duplicates. A CEL gate expression like `artifacts.review_plan.summary != ""` would evaluate against whichever artifact happened to iterate last in the Map — producing silently wrong gate results. The same applies to evidence names (`gates.ts:354-356`) and approval gate IDs (`gates.ts:359`). **Breaking input:** A definition with artifacts `code-review` and `code_review` on different stages. A CEL gate referencing `artifacts.code_review` reads the wrong payload. **Suggested fix:** Add a celName-uniqueness check to `validateGraph` — after collecting all artifact names, also build a `Set<string>` of `celName(name)` values and error on collision. 2. **`artifact_schema.ts:55` — ReDoS via user-controlled regex pattern in artifact schemas.** ```typescript if (decl.pattern !== undefined) s = s.regex(new RegExp(decl.pattern)); ``` The `pattern` field from a factory definition's artifact schema is compiled into a `RegExp` without any complexity check. A pathological pattern like `(a+)+$` causes catastrophic backtracking when matching certain inputs. This runs both at definition validation time (`compileArtifactSchema` via `validateGraph`) and at payload recording time (`validateArtifactPayload`). A crafted payload like `"aaa...aab"` against such a pattern would block the engine thread. **Breaking input:** An artifact declares `schema: { type: "string", pattern: "(a+)+$" }`, then `record_artifact` is called with payload containing `"aaaaaaaaaaaaaaaaaaaaaaaaaaab"`. **Suggested fix:** Either impose a max pattern length or set a timeout/complexity limit. Alternatively, wrap `new RegExp(decl.pattern)` in a try-catch at compilation time (it already is, in `validateGraph`), and consider using a regex-safe library or at minimum documenting the operator trust boundary. ### Low 3. **`gates.ts:309` — `cooldown` gate silently passes on corrupted `recordedAt` timestamp.** ```typescript const elapsed = (ctx.now.getTime() - new Date(recordedAt).getTime()) / 1000; if (elapsed < gate.config.seconds) { /* fail */ } return pass(gate); ``` If `recordedAt` is not a valid ISO date, `new Date(recordedAt).getTime()` returns `NaN`, making `elapsed` `NaN`. Since `NaN < anything` is `false`, the cooldown check is skipped and the gate passes unconditionally. This only matters with corrupted data (the engine always writes `new Date().toISOString()`), but a cooldown gate is a safety control — failing open is the wrong default. **Suggested fix:** Add `if (Number.isNaN(elapsed)) return fail(gate, "invalid timestamp on ...")` before the comparison. 4. **`gates.ts:498-504` — unescaped string interpolation in workflow-succeeded query predicates.** ```typescript outputs = await ctx.queryData( `workflowRunId == "${summary.workflowRunId}" && name == "${physical}"${selfClause}`, ); ``` `summary.workflowRunId` is parsed from JSON content stored in the data repository. If that content contains a `"` character (corrupted or crafted data), it breaks the CEL predicate syntax or alters its semantics. Attack surface is limited to compromised platform data — not user-facing input. **Suggested fix:** Escape double-quotes in interpolated values, or use a parameterized query interface if the platform supports one. 5. **`software_factory.ts:1086-1176` — non-atomic read-modify-write in `resolve_findings`.** `resolve_findings` reads the current artifact, mutates the findings array, and writes back. Two concurrent calls targeting the same artifact would each read the same version, apply their own resolutions, and write — the last write wins, silently dropping the other's resolutions. The model code does not use optimistic concurrency (e.g., compare-and-swap on version). **Suggested fix:** Either version-check before writing (fail if the version changed since read) or document that concurrent `resolve_findings` calls on the same artifact are unsupported. ### Verdict **PASS** — The code is well-structured with thorough validation, defense-in-depth gate re-evaluation, and good error messages. The findings above are edge cases: the CEL name collision (#1) and ReDoS (#2) are the most worth addressing before this ships, but neither is exploitable from the normal driver/user path since definitions are operator-authored. No critical or high severity issues found.

adamhjk merged commit fe694a3669 into main 2026-06-16 21:03:40 +00:00

adamhjk deleted branch software-factory 2026-06-16 21:03:41 +00:00

adamhjk referenced this pull request from a commit 2026-06-16 21:03:41 +00:00

feat(software-factory): add @swamp/software-factory extension (#58)

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

swamp-club/swamp-extensions!58

No description provided.