Skip to content

fix(unic-archon-dlc): explicit staging, and composed repository pinning in the Archon Boxes - #308

Merged
orioltf merged 5 commits into
developfrom
archon/task-feature-unic-archon-dlc-289-stage-explicitly-compo
Aug 10, 2026
Merged

fix(unic-archon-dlc): explicit staging, and composed repository pinning in the Archon Boxes#308
orioltf merged 5 commits into
developfrom
archon/task-feature-unic-archon-dlc-289-stage-explicitly-compo

Conversation

@orioltf

@orioltf orioltf commented Aug 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #289.

Why

Two defects, both present in the Boxes since they were written, found by reading the Archon 0.7.0 release against our own workflows.

Blind staging. /explore's preserve-spike ran git add -A; /build's open-pr said "stage everything changed by the build". A Box runs in an isolated worktree with fresh context, so it cannot tell its own output from whatever else is on disk — and "everything changed" includes build-state.json, which the build loop rewrites on every one of up to 60 iterations.

Unpinned repository. No PR or tracker call named the repository it acted on, so a host tool inferred it from the checkout. In a fork clone that is the parent, and the PR opens on someone else's project.

A first attempt (#307, closed unmerged) fixed the second defect by prescribing the flag per host, and so wrote 22 --repository, 16 gh pr create and 11 az repos pr occurrences into seven prompt blocks — including one az repos pr thread subcommand that does not exist. That is what ADR-0016 forbids and what this PR is shaped to prevent: an agent composing a call at run time reads the tool as it is today, whereas a flag table frozen in a YAML file is stale the moment the tool changes, cannot be verified without a live tenant, and is copied as precedent by the next agent that reads it.

What changed

Staging (AC 1–5, 8). Every committing node stages a named list, one git add <path> per path, then confirms with git status --porcelain and unstages anything else. /build's open-pr list is explicit — source, tests, PRD.md, issues.json, report.md, build-state.json, each drafted docs/adr/NNNN-*.md. /explore's spike node now records the paths it wrote so preserve-spike can name them. /specs and /tickets stage named paths in both the open-pr and stage-only gates, and no longer git add docs/adr/ as a directory. The deny list (pr-body.md, *.tmp.md, *.scratch.md, anything under $ARTIFACTS_DIR) is inline in each node, not doctrine: an Archon node imports nothing from the Plugin (ADR-0023 §5), so a rule that lives only in AGENTS.md is invisible at run time.

build-state.json (AC 3). Committed once, at open-pr; the loop is told explicitly never to stage it. That commit is the durable proof of ADR-0012's anti-cheat record, which previously died whenever /cleanup pruned the worktree.

Repository pinning (AC 6, 7). Each Box's bootstrap derives the repository from the worktree's origin remote. project.repo_ref is an optional override, absent from the default config, so no existing Consumer config breaks. A new guard-ambiguous-repo node cancels — not fails (ADR-0011) — only when the checkout names more than one repository and no override is set; a checkout with a single origin never reaches it. Every PR-touching prompt states the invariant "act on THIS repository, never the one a tool infers from the checkout", names the derived repository, and stops if the registered system-skill cannot target a repository explicitly.

No provider knowledge (AC 6). Ten shipped files lose their per-provider branches. They name the config keys (tracker.type, tracker.access) and compose the skill the team registered, reading that skill's own current interface. Behaviour that used to be hardcoded per host — is there a queryable PR, can it comment on a file and line, can it merge — is now a capability the composed skill is asked about, degrading to summary-only or manual steps when the answer is no.

The barrier (AC 10). test/box-staging-and-repo-pinning.test.mjs greps the four Box YAMLs, both interactive command docs and the four Archon command docs — AC 10 names the first six; the stubs are in because AC 6's rule is "no Box prompt or command doc", and a Consumer's agent reads a stub as readily as a prompt. On a hit it fails naming every token by file:line. It self-tests its own patterns so a mistyped regex cannot fail open, checks the guarded files are non-empty so a rename cannot reduce coverage to zero, and asserts the positive rules too: named-path staging, the deny list, the derived repository, the two mutually-exclusive guards, and the invariant per PR-touching node. Proven to fail on a synthetic violation before being trusted.

commands/setup.md is deliberately not guarded: /setup is the one surface that legitimately holds provider values, because it conducts the conversation that writes tracker.type.

Verification

Check Result
pnpm --filter unic-archon-dlc test ✅ 220 pass, 0 fail
pnpm --filter unic-archon-dlc typecheck ✅ exit 0
pnpm ci:check ✅ exit 0
pnpm --filter unic-archon-dlc verify:changelog ✅ ok

Version bumped to 0.15.1 via pnpm bump patch, with a dated CHANGELOG entry.

Note for the reviewer

.archon/commands/unic-dlc-build.md and unic-dlc-pr-review.md still describe a REFACTOR phase and seven aspect nodes respectively — both retired by #281. That staleness is pre-existing and out of this ticket's scope; only their provider tokens were touched here.

Scope note (self-fix, code-review Finding 1)

The guard-ambiguous-repo node and repo_ref/repo_ref_source derivation were extended to /explore as well as /build, /qa, and /pr-review, even though the review scope's OUT-OF-SCOPE list named this deferred. This was a deliberate widening during implementation for symmetry — every Box that writes to a repository gets the same guard — not an unscoped addition. Code, CHANGELOG ("every Box"), and the new test's WORKFLOWS constant all agree with each other on this boundary; they just disagree with the scope artifact. Noting here per the code-review agent's Option A recommendation rather than reverting the guard.

🤖 Generated with Claude Code

orioltf added 3 commits August 5, 2026 22:07
@orioltf
fix(unic-archon-dlc): explicit staging, and composed repository pinni…
8f2420c 
…ng in the Archon Boxes

Two defects, both present since the Boxes were written.

`/explore`'s `preserve-spike` ran `git add -A` and `/build`'s `open-pr` said
"stage everything changed by the build". A Box runs in an isolated worktree with
fresh context, so "everything changed" sweeps in whatever else is on disk —
including `build-state.json`, which the build loop rewrites on every one of up to
60 iterations.

No PR or tracker call named the repository it acted on, so a host tool inferred
it from the checkout. In a fork clone that is the parent, and the PR opens on
someone else's project.

Changes:
- Every committing node stages a named list, one `git add <path>` per path, then
  confirms with `git status --porcelain`. The deny list (`pr-body.md`, `*.tmp.md`,
  `*.scratch.md`, `$ARTIFACTS_DIR`) is stated inline in each, because an Archon
  node imports nothing from the Plugin (ADR-0023 §5).
- `build-state.json` is committed once, at `open-pr`, never in a loop iteration —
  it is the durable proof of ADR-0012's anti-cheat record.
- Every Box's bootstrap DERIVES the target repository from the worktree's `origin`
  remote. `project.repo_ref` is an optional override, absent by default, so no
  existing config breaks. A new `guard-ambiguous-repo` node CANCELS only when the
  checkout names more than one repository and no override is set (ADR-0011).
- No prompt or command doc carries a host CLI token, subcommand, flag, or provider
  name. Each states "act on THIS repository, never the one a tool infers from the
  checkout" and composes the skill registered under `tracker.access`, reading that
  skill's own current interface (ADR-0016).
- New `test/box-staging-and-repo-pinning.test.mjs` greps ten shipped files for CLI
  and provider tokens and fails loudly with file:line. It self-tests its own
  patterns, so a mistyped regex cannot fail open.

Fixes #289
@orioltf
docs(unic-archon-dlc): changelog entry for the staging and repo-pinni…
56c9657 
…ng fix
@orioltf
chore(unic-archon-dlc): bump to 0.15.1
5c76bb9
@orioltf
orioltf changed the base branch from develop to main August 5, 2026 20:14
@orioltf
orioltf changed the base branch from main to develop August 5, 2026 20:15
@orioltf

orioltf commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

🔍 Comprehensive PR Review

PR: #308
Reviewed by: 4 specialized agents (comment-quality artifact missing this run — see note)
Date: 2026-08-05

Summary

Prompt/config-only PR: explicit named-path staging replaces every blind git add -A/./docs/adr/ sweep across the four Archon Box YAMLs, and repository targeting is now derived from origin (with an optional project.repo_ref override) instead of a host tool's inference. The new test/box-staging-and-repo-pinning.test.mjs (380 lines) mechanically enforces both rules plus the zero-provider-knowledge barrier across all ten guarded surfaces, self-tests its own regexes, and asserts the positive rules too. All four reporting agents independently verdict APPROVE.

Verdict: APPROVE

Severity Count
🔴 CRITICAL 0
🟠 HIGH 0
🟡 MEDIUM 0
🟢 LOW 4

⚠️ Note: comment-quality-findings.md was not produced this run — that dimension is unreviewed, not "0 findings."

🟢 Low Issues

View 4 low-priority items
Issue Location Agent Suggestion
/explore gained the ambiguous-repo guard the scope doc listed as deferred unic-dlc-explore.yaml:568-575 code-review Not a defect — code/tests/changelog agree with each other, just not with the scope artifact. Add a one-line note to #289 that the guard was extended to /explore for symmetry during implementation.
Two nodeSource() call sites skip the assert.ok existence check used everywhere else test/box-staging-and-repo-pinning.test.mjs:301-313 error-handling Add assert.ok(guard, …) / assert.ok(guardNotReady, …) after each extraction — mechanical, zero behaviour change.
The 4 .archon/commands/*.md stubs are grepped for banned tokens but not for the repository-invariant sentence test/box-staging-and-repo-pinning.test.mjs:318-366 test-coverage Likely intentional — stubs are thin pointers by doctrine, YAML is the enforcement point. No action needed.
CONTEXT.md Language section has no entry for the new repository-derivation vocabulary unic-archon-dlc/CONTEXT.md docs-impact Add a "Repository derivation" entry mirroring the existing Container/config.yaml entries, linking ADR-0011.

✅ What's Good

  • New test file self-tests its own banned-token regexes against synthetic samples before trusting them against real files.
  • GUARDED.length === 10 asserted as hard equality — a dropped file fails loudly instead of coverage silently shrinking.
  • Every PR/tracker-writing node across all four Boxes cancels/blocks rather than silently falling back to an inferred repository — the exact failure mode this PR exists to close.
  • guard-ambiguous-repo correctly cancel:s, not fails (ADR-0011), and is verified mutually exclusive with guard-not-ready.
  • AGENTS.md, README.md, CHANGELOG.md already updated in this diff; root-level docs correctly left untouched.
  • pnpm test (220/220), typecheck, ci:check, verify:changelog all green; CI 9/9 passing.

Next Steps

  1. No CRITICAL/HIGH issues — nothing blocks merge.
  2. Optional: apply the assert.ok guard fix and the CONTEXT.md vocabulary entry — both have ready-to-paste diffs in the agent artifacts.
  3. Optional: one-line scope note on fix(unic-archon-dlc): explicit staging and host-agnostic repo pinning in the Archon Boxes #289 re: /explore's guard extension.
  4. Follow up on why comment-quality-findings.md wasn't produced this run.

Reviewed by Archon comprehensive-pr-review workflow
Artifacts: /Users/oriol.torrent/.archon/workspaces/unic/unic-agents-plugins/artifacts/runs/30d57c49d256eb84093da593538b3986/review/

@orioltf
fix(unic-archon-dlc): address PR #308 review findings
799fe4e 
Add assert.ok guards after the two unchecked nodeSource() extractions in
box-staging-and-repo-pinning.test.mjs, matching the file's own convention, so
a future extraction failure surfaces a descriptive message instead of a bare
TypeError. Add a CONTEXT.md Language entry for repository derivation — the
vocabulary AGENTS.md's new doctrine bullet coins but the Language section
didn't carry.

Fixed:
- Two nodeSource() call sites (guard-ambiguous-repo, guard-not-ready) now assert.ok before use
- CONTEXT.md Language section gained a "Repository derivation" entry, linking ADR-0011

Skipped:
- test-coverage Finding 1 (stub .md files not checked for repo-invariant wording): reviewer's own assessment marks this intentional by design — stubs are thin pointers, the YAML is the enforcement point
@orioltf

orioltf commented Aug 5, 2026

Copy link
Copy Markdown
Member Author

⚡ Self-Fix Report (Aggressive)

Status: COMPLETE
Pushed: ✅ Changes pushed to archon/task-feature-unic-archon-dlc-289-stage-explicitly-compo (799fe4e)
Philosophy: Fix everything unless clearly a new concern

Fixes Applied (3 total)

Severity Count
🔴 CRITICAL 0
🟠 HIGH 0
🟡 MEDIUM 0
🟢 LOW 3
View all fixes
  • ✅ Two nodeSource() call sites skip the assert.ok existence check (test/box-staging-and-repo-pinning.test.mjs:301-313) — added assert.ok after both extractions, matching the file's own convention (error-handling Finding 1)
  • ✅ CONTEXT.md Language section missing repository-derivation vocabulary (CONTEXT.md) — added a "Repository derivation" entry mirroring the existing Container/config.yaml format, linking ADR-0011 (docs-impact Finding 1)
  • ✅ /explore gained the ambiguous-repo guard the scope doc listed as deferred — not a code defect; added a scope note to the PR description per the reviewer's own recommended Option A, rather than reverting (code-review Finding 1)

Tests Added

(none) — fix was an existence assertion on an existing test path; 220/220 tests still pass.

Skipped (1)

Finding Reason
Stub .md files not checked for repo-invariant wording (test-coverage Finding 1) New concern only in the sense that the reviewing agent's own write-up called it intentional by design — stubs are thin pointers per this Plugin's doctrine, the YAML is the enforcement point. Extending the check would encode a paraphrase the doctrine forbids.

Suggested Follow-up Issues

(none)

Validation

✅ Type check | ✅ Lint (pnpm ci:check) | ✅ Tests (220 passed)

Self-fix by Archon · aggressive mode · fixes pushed to archon/task-feature-unic-archon-dlc-289-stage-explicitly-compo

@orioltf orioltf mentioned this pull request Aug 5, 2026
Closed
@orioltf
orioltf requested a lite review from Copilot August 10, 2026 07:20
Copilot AI reviewed Aug 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the unic-archon-dlc Archon workflows (“Boxes”) by enforcing explicit staging and deriving the target repository from the worktree origin, while introducing a regression test barrier that prevents provider-/CLI-specific tokens from reappearing in guarded prompts and command docs.

Changes:

  • Replaced blind staging guidance with explicit, named-path staging rules (including deny-lists and staged-set verification).
  • Added repository derivation + an “ambiguous repository” cancel guard across the Archon workflows and updated interactive command docs to derive TARGET_REPO.
  • Added a new barrier test that scans guarded workflows/docs for provider knowledge and asserts the positive staging/repo-derivation invariants.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
apps/claude-code/unic-archon-dlc/test/config-schema.test.mjs Adds tests ensuring project.repo_ref is absent by default and preserved when overridden.
apps/claude-code/unic-archon-dlc/test/command-methods.test.mjs Extends harness doc token allowlist (incl. pr-body.md) to support new deny-list prose.
apps/claude-code/unic-archon-dlc/test/box-staging-and-repo-pinning.test.mjs New barrier test enforcing “no provider knowledge”, explicit staging, and repo derivation rules.
apps/claude-code/unic-archon-dlc/test/archon-box-methods.test.mjs Adjusts comments to reflect provider-agnostic wording and staging-rule references.
apps/claude-code/unic-archon-dlc/README.md Documents project.repo_ref as an optional override and describes derived-repo behavior.
apps/claude-code/unic-archon-dlc/package.json Bumps version to 0.15.1 and adds the new test to the test script.
apps/claude-code/unic-archon-dlc/CONTEXT.md Adds vocabulary/notes for repository derivation and guard behavior.
apps/claude-code/unic-archon-dlc/commands/specs.md Adds TARGET_REPO derivation section and updates PR gate staging + provider-agnostic PR-opening instructions.
apps/claude-code/unic-archon-dlc/commands/tickets.md Adds TARGET_REPO derivation section and updates staging + provider-agnostic publishing/PR-opening instructions.
apps/claude-code/unic-archon-dlc/commands/setup.md Clarifies that project.repo_ref is not asked/written by default (override only on request).
apps/claude-code/unic-archon-dlc/CHANGELOG.md Adds 0.15.1 entry describing staging hardening, derived repo, and the barrier test.
apps/claude-code/unic-archon-dlc/AGENTS.md Adds a load-bearing invariant capturing explicit staging + derived repository rules and the barrier test.
apps/claude-code/unic-archon-dlc/.claude-plugin/plugin.json Bumps plugin version to 0.15.1.
apps/claude-code/unic-archon-dlc/.claude-plugin/marketplace.json Bumps marketplace version to 0.15.1.
apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-build.yaml Derives repo in bootstrap, adds ambiguous-repo cancel guard, and makes open-pr staging explicit (incl. build-state handling).
apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-explore.yaml Derives repo in bootstrap, adds ambiguous-repo cancel guard, and stages spike artifacts by recorded paths instead of sweeping.
apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-pr-review.yaml Derives repo in bootstrap, adds ambiguous-repo cancel guard, and makes PR-touching nodes explicitly repository-scoped/provider-agnostic.
apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-qa.yaml Derives repo in bootstrap, adds ambiguous-repo cancel guard, and scopes PR/issue operations to the derived repository.
apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-build.md Updates Archon command stub to reflect derived repository + explicit staging changes.
apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-explore.md Updates Archon command stub to reflect derived repository + named-path spike preservation.
apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-pr-review.md Updates Archon command stub language around intent sources and provider-agnostic targeting.
apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-qa.md Updates Archon command stub to reflect derived repository + explicit repo naming for PR/issue operations.
Suppressed comments (2)

apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-pr-review.md:12

  • This overview still describes the /pr-review workflow as fanning out seven aspects, but the workflow YAML now documents a two-axis review (Standards + Spec). Leaving this as-is will mislead operators reading the Archon command stub.

This issue also appears on line 44 of the same file.

Runs the `pr-review` box: composes a shared **Intent Brief** (from the linked work items, the referenced
docs pages, the PR description, and `PRD.md`), fans out **seven review aspects** as parallel fresh nodes
(code-quality, test-coverage, silent-failure, type-design, comment-rot, code-simplification, and an
intent/AC-coverage check), synthesises the findings, **reconciles them against the prior iteration**
(new / still-present / fixed / regressed), and — after a config-gated human confirm — posts or updates a
single structured **summary comment** plus **inline comments** on the current PR.

apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-pr-review.md:48

  • This step list still claims there are "7 aspect nodes" with spawn gates, which no longer matches the current /pr-review workflow description (two-axis Standards/Spec fan-out). Consider updating this section so the command doc reflects the current node structure.
3. **7 aspect nodes** (parallel, fresh) — each reads the shared Intent Brief (**every aspect is
   intent-grounded**) + the diff and emits findings scored on the **confidence rubric** (90–100 Critical
   / 80–89 Important / 60–79 Minor / below the threshold dropped). **Spawn gates** run each aspect only
   when meaningful: code-quality + intent-check always; tests/type-design/comment-rot/simplifier/
   silent-failure gated on the changed-file categories.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread apps/claude-code/unic-archon-dlc/test/box-staging-and-repo-pinning.test.mjs Outdated
@orioltf @claude
fix(unic-archon-dlc): close the directory-sweep bypass, and unstale t…
9655f8d 
…he /pr-review stub

Three Copilot findings on PR #308, all confirmed by running them.

The directory-sweep guard was anchored to end-of-line, so it only ever saw
`git add docs/adr/` alone on its line. Four spellings walked past it: a trailing
comment, a quoted path, an `&&` chain, and `git add -- docs/adr/` — which is the
`git add -- "<path>"` form these prompts themselves prescribe, so a sweep written
in the house style was invisible. Replaced with a token-wise reader, self-tested
against all seven sweep spellings and three named-path forms it must stay silent
on. Injecting the `--` spelling into unic-dlc-build.yaml now fails the barrier;
the old pattern reported the same file clean.

The /pr-review command stub still advertised "seven review aspects" with spawn
gates in its frontmatter, its overview and its step list. #281 collapsed those
seven nodes into one `review` node hosting the code-review Method's own two-axis
Standards/Spec fan-out (unic-dlc-pr-review.yaml:233), and the categories in `prep`
no longer gate anything. This PR had already edited that sentence for
provider-agnostic wording and left the wrong claim standing beside the new words.

Verified at this commit: 220/220 tests, typecheck and ci:check all exit 0.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MgKAGuqw6tnVZqHzPa3xC3
@orioltf
orioltf merged commit eb17c40 into develop Aug 10, 2026
9 checks passed
@orioltf
orioltf deleted the archon/task-feature-unic-archon-dlc-289-stage-explicitly-compo branch August 10, 2026 07:55
This was referenced Aug 10, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix(unic-archon-dlc): explicit staging and host-agnostic repo pinning in the Archon Boxes

2 participants

@orioltf