Merged
Conversation
Add the design note for the per-repository agent-maintained knowledge system under docs/notes/agent-knowledge-system-design.md. This captures the full design discussion — problem, karpathy three-layer mapping, hot path (real-time ingest) vs cold path (scheduled sweep), capture / consume loop, MCP vs CLI layering, config surface, invariants, phasing, and key decisions — so the requirements can be read alongside the reasoning that produced them. Add the implementation order list under docs/notes/agent-knowledge-system-order.md. One requirement UID per line, in the order they should be implemented based on the DEPENDS_ON / REFINES edges recorded in Ground Control. The requirements themselves (GC-X001 through GC-X025) live in Ground Control. X001 is the umbrella; the 24 children are linked via PARENT to X001 and connected to each other via DEPENDS_ON and REFINES to encode the real implementation dependencies so a future planner can walk the graph without re-deriving order.
Previously /implement took a single Ground Control requirement UID and
derived everything from it — the issue was a side effect created behind
the scenes during Step 1. That model forced every change to be modeled as
a requirement (bug fixes, refactors, dependency bumps and other
requirement-free work had no natural entry point) and could not express
grouped implementation (shipping multiple related requirements in one PR
with coherent review boundaries).
New model: every /implement run is driven by a GitHub issue, and the
issue body declares which requirements are in scope via a conventional
`## Requirements` markdown section containing a bulleted list of UIDs.
Three cases fall out:
- Empty or missing `## Requirements` section: bug fix / refactor /
maintenance. Zero requirements are transitioned. Traceability is
still reconciled against the diff, because a bug fix may touch files
linked to other requirements whose links are now stale.
- One UID: equivalent to the old single-requirement flow, just entered
via an issue.
- N UIDs: grouped implementation. Step 4.5 clause verification iterates
the union of clauses across all in-scope requirements. Step 15
reconciles traceability for all of them. Step 16 transitions each to
ACTIVE. Step 17 verifies.
Backwards-compatible invocation: `/implement GC-X042` still works. Step 1
detects the input shape, and if it's a UID, finds or creates the matching
issue via existing traceability (or `gc_create_github_issue` if none
exists yet) and treats that issue as the authoritative input. The UID
becomes a single entry in `in_scope_requirements[]`.
Step 15 is also rewritten from "create links for this requirement" to
"reconcile the Ground Control graph against the diff". It walks every
added/modified/renamed/deleted file, finds existing links, and decides
per-file whether to leave the link alone, update the path, create a new
link for behavior that moved, delete a stale link, or flag a coverage
regression for user escalation (when the sole implementation of a
requirement is being deleted). Reconciliation is idempotent: running it
on a branch where the graph already matches is a no-op.
docs/DEVELOPMENT_WORKFLOW.md updated:
- The workflow heading is now `/implement <issue-number | requirement-uid>`.
- A new explanation of the two invocation modes, grouped implementation,
and the `## Requirements` section convention.
- The mermaid flow diagram updated to show issue-first entry (step 1
"Resolve issue + parse Requirements section"), Step 17 as
"Reconcile traceability against diff" rather than "Create traceability
links", and Step 18 as "Transition in-scope requirements DRAFT → ACTIVE"
(zero or more).
- Narrative paragraphs rewritten to call out that reconciliation applies
regardless of in-scope requirement count, and that a bug fix still
updates the graph so drift doesn't accumulate.
No code changes — this is entirely a skill + docs change. The existing
MCP surface (`gc_get_traceability_by_artifact`, `gc_create_traceability_link`,
`gc_delete_traceability_link`, `gc_transition_status`, `gc_get_requirement`,
`gh issue view`, `gh issue develop`) is sufficient for the new flow.
Extend .ground-control.yaml with an optional `knowledge` section declaring the per-repo knowledge base location. `knowledge.dir` is required; `schema` and `inbox` are optional overrides that default to <dir>/SCHEMA.md and <dir>/inbox. All knowledge paths are validated against the repository root via a new shared resolver that rejects absolute paths and `..` traversal. `gc_get_repo_ground_control_context` now surfaces a resolved `knowledge` block when the section is configured, with existence checks for the directory and schema file. The inbox path is surfaced but not existence- checked; later slices will create it on first capture. Commit the docs/knowledge/ skeleton (SCHEMA.md, index.md, log.md) and wire this repo's own .ground-control.yaml to point at it. Fix stale AGENTS.md wording in mcp/ground-control/README.md that described the tool as reading from AGENTS.md; the tool reads .ground-control.yaml. Implements GC-X002, GC-X003, GC-X004, GC-X005, GC-X013.
- lib.js: reject symlink-based escapes in knowledge.dir / schema / inbox via realpath containment check in addition to the existing lexical check. Canonicalizes the repo root once per request and walks up nonexistent ancestors so a symlink on any parent of inbox is still caught even when inbox itself is pending creation. - lib.js / index.js: make requirement_uid optional on gc_codex_architecture_preflight and require at least one of requirement_uid or issue_number. The prompt builder switches to a requirement-free preamble when requirement is null so issue-first runs (bugs, refactors, maintenance) can reach architecture preflight without manufacturing a fake UID. - lib.js: formatIssueBody now seeds a ## Requirements section with the UID → title bullet so /implement's issue-first parser finds in_scope_requirements[] on the round-tripped issue. - SKILL.md Step 2.5: spell out the UID-optional call signature and the issue-number-only fallback. - SKILL.md Step 15.1: document the origin/dev → dev → origin/main → main → git fetch fallback for the reconciliation base ref, matching the codex review diff resolver. - SKILL.md Step 15.6: reconciliation now deletes stale issue→requirement links for requirements removed from the issue body, not just adds missing ones. Catches the case where an issue was narrowed after the initial link set was created. Tests: +8 cases covering symlink dir escape, symlink schema escape, symlink-escaping inbox parent, in-repo symlink happy path, preflight prompt without a requirement, preflight prompt completion line selection, formatIssueBody Requirements section seeding, and the title-less formatIssueBody fallback. 88 tests pass, 0 new lint warnings.
- lib.js: sanitize requirement titles before emitting the
## Requirements bullet in formatIssueBody. A multiline title could
previously inject a second list item that /implement would parse as
an unrelated UID in in_scope_requirements[]. Collapsing whitespace
runs to single spaces forces the bullet onto one line.
- lib.js: runCodexArchitecturePreflight fails fast on issue-only runs
when the GitHub issue body could not be loaded. getIssueContext
previously swallowed lookup failures and returned {number, warning},
so a wrong repo slug or missing GH_REPO would let preflight proceed
against an empty spec.
- lib.js: getIssueContext now accepts a cwd option and
runCodexArchitecturePreflight passes repoRoot so gh resolves the
target repository from the checkout's own git config when no
explicit repo was supplied.
- SKILL.md Step 15.5: rewrite the coverage rule so documentation,
configuration, and workflow requirements are not forced to produce
production code and automated tests. IMPLEMENTS is still required
for every in-scope requirement, but it can point at an ADR, SCHEMA
page, config file, workflow file, or hook — whichever form of
implementation the requirement actually demands. TESTS is only
required when the requirement has testable behavior and the diff
introduces or touches a verifying test. The no-fabrication and
no-wrong-link guardrails are made explicit so the agent cannot
paper over a gap by linking to a plausible-looking neighbor.
Tests: +2 formatIssueBody cases (multiline title injection, tab /
whitespace collapse). 90 tests pass, 0 new lint warnings.
Raise the review cycle cap from 2 to 5 in the /implement and /ship skills to reflect that the Claude-hosted review steps were removed; only codex + review-tests remain, so each surviving review gets more headroom to converge. lib.js defensive fixes: - runCodexArchitecturePreflight no longer aborts issue-only runs on valid title-only issues. GitHub returns body: "" for these, and the old check (!issueContext.body) treated empty string like a failed lookup. The check now tests for the warning field or a missing body property (not a falsy body value). - resolveKnowledgeBlock now rejects knowledge.inbox values that point at an existing regular file. Before this fix, inbox: docs/knowledge/SCHEMA.md slipped through lexical + realpath containment and landed as status: ok, silently breaking every future capture flow. Nonexistent inboxes remain the happy-path state — later slices create the directory on first capture. - assertRealpathInRepo now walks up past ENOTDIR the same way it walks up past ENOENT. Before this fix, inbox: docs/knowledge/ SCHEMA.md/capture let realpathSync throw ENOTDIR and the exception escaped the MCP tool, hard-failing gc_get_repo_ground_control_context instead of returning the documented invalid_ground_control_yaml envelope. Non-EACCES, non-ENOENT, non-ENOTDIR errors still escape — those are real I/O failures, not config drift. SKILL.md workflow fixes: - /implement Step 2.5 now preflights every UID in in_scope_requirements[], not just the first. Grouped issues that carry multiple UIDs (like #522 with its five GC-X00* entries) were previously only seeing preflight guardrails for UID #1, and every requirement after the first ran without architecture preflight. - /implement Step 15.5 now has two modes: Mode A is the common case where the diff implemented the work (unchanged behavior, every requirement needs IMPLEMENTS against a touched file); Mode B is the reconciliation-only path where Step 4 concluded the work is already done and the graph just needs repair. Mode B accepts existing IMPLEMENTS coverage instead of forcing a no-op source edit, and backfills missing links against existing files in the repo when a requirement has zero existing coverage. Hard-fails when no implementing file exists anywhere — that's a requirement that should be DEPRECATED, not silently linked. Tests: +2 cases for knowledge.inbox pointing at a file, and for inbox descending through a regular file (ENOTDIR path). 92 tests pass, 0 new lint warnings.
Step 15 was "Reconcile Traceability Links" and Step 16 was "Transition
In-Scope Requirements to ACTIVE". That order does not actually work —
the Ground Control API enforces `IMPLEMENTS → ACTIVE` and returns
`422 requirement_not_active` for every `gc_create_traceability_link`
call with `link_type: IMPLEMENTS` against a DRAFT requirement. A run
following the documented order silently fails 10+ parallel link
creations before it realizes the wall; I hit that and had to reorder
on the fly on this branch.
Swap the two steps so the order matches what the API accepts:
- Step 15 is now "Transition In-Scope Requirements to ACTIVE" — the
transition is framed semantically as the commit-to-contract moment
(code exists, reviews are green, the requirement is no longer a
proposal) rather than a post-traceability bookkeeping chore.
- Step 16 is now "Reconcile Traceability Links Against the Diff" —
the intro explains that reconciliation must run AFTER transition
because the API gate would otherwise reject every IMPLEMENTS call.
- Step 17 ("Verify Ground Control State Landed") cross-references
updated to point at Step 16 for link drift and Step 15 for status
drift. Added a fourth point explicitly prohibiting silent-failure
tolerance: if any link-create or transition call returns non-2xx,
treat it as broken and loop back, do not barrel past it.
- Earlier step 1.10/1.11/4-report cross-references to "Step 15" /
"Step 16" updated to match the swap.
Step cross-references in the rest of the skill already used Step 14
(final CI re-verification) and Step 18 (report), so those are
unchanged.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First slice (issue #522) of the six-slice agent knowledge system rollout.
.ground-control.yamlwith an optionalknowledgesection (dirrequired,schemaandinboxoptional overrides). Paths are validated against the repo root via a shared resolver that rejects absolute paths and..traversal.gc_get_repo_ground_control_contextnow surfaces a resolvedknowledgeblock with existence checks for the directory and schema file. The inbox path is surfaced but not existence-checked; later slices create it on first capture.docs/knowledge/skeleton:SCHEMA.md(conventions, source-citation rule, one-repo invariant, flat-navigation rule),index.md(empty content catalog), andlog.md(append-only history)..ground-control.yamlto point atdocs/knowledge/.AGENTS.mdwording inmcp/ground-control/README.mdso it describes the tool as reading.ground-control.yaml(which it always did)./implementissue-first entry point with traceability reconciliation, and the bootstrap-claude-workflow hook/skill copy mode.Requirement UIDs
ADR Impact
No ADR required. This slice is purely config wiring plus a documentation skeleton — no new architectural decisions. The agent knowledge system's overall design is captured in
docs/notes/agent-knowledge-system-design.mdand does not yet warrant an ADR; a future slice that introduces thegc_rememberprimitive and ingest engine will be the right moment to promote that design note to an ADR if the architecture has stabilized by then.Ground Control Checks
make policypassesgc_evaluate_quality_gatespasses or is unchanged by this repo-only changegc_run_sweepreviewed or intentionally deferred with reasongc_evaluate_quality_gatesis unchanged by this slice — no new requirements, no new ADRs, no new code coverage gates introduced.gc_run_sweepis deferred: no new projects, baselines, or sweep watermarks are touched, so a sweep pass is a no-op for this change.Traceability
mcp/ground-control/lib.js,docs/knowledge/SCHEMA.md,docs/knowledge/index.md,docs/knowledge/log.md,.ground-control.yaml→ GC-X002, GC-X003, GC-X004, GC-X005, GC-X013mcp/ground-control/lib.test.js→ GC-X002, GC-X004, GC-X013 (parser and context-loader behavior; GC-X003 and GC-X005 are documentation invariants not covered by unit tests)mcp/ground-control/README.md,docs/notes/agent-knowledge-system-design.md→ GC-X002Closes #522.