[issue-3077][slice-2/6] Artifact spec module: single declarative... - #3141
Conversation
…2 TASK-2-1) Add `shared/egg_contracts/artifact_spec.py`: a frozen `ArtifactSpec` dataclass plus a tuple registry covering the five refine/plan coordination artifacts (analysis-draft, plan-draft, architect-output, architect-slices, risk-analyst-output) with `resolve_artifact_path`, `specs_for`, `spec_by_name`, and `all_specs` helpers. Path templates mirror today's conventions (`orchestrator.routes.pipelines._get_draft_path` + the `.egg-state/agent-outputs/` filenames) so slice-3 can subsume `_validate_producer_draft_present` and `_get_draft_path` without a behavioural change. Pure Python; no orchestrator or gateway imports.
Add `shared/egg_contracts/tests/test_artifact_spec.py` — the refine-risk-1 mitigation from #3077: every path replica that admits or constructs a coordination artifact must stay in lockstep with the new `egg_contracts.artifact_spec` registry, or CI fails here. Coverage matches the `task-2-2` contract: - (a) every spec path (both identifier shapes) is admitted by `gateway.phase_filter.PhaseFilter.check_phase_file_restrictions` AND `egg_restrictions.phase_patterns.phase_file_verdict`; - (b) `_get_draft_path(refine|plan, …)` byte-equals `resolve_artifact_path("analysis-draft"|"plan-draft", …)` for both identifier shapes (`int` issue number / `str` pipeline id); - (c) every f-string draft-path literal in `orchestrator/routes/pipelines.py` corresponds to a registered spec row, and every registered `agent-outputs/` row appears as a literal — drift either direction fails CI. - Plus resolve/round-trip/`spec_by_name` cases for every registered row, registry-shape assertions (frozen rows, single producer, non-empty consumers, `{identifier}` placeholder), unknown-name `raises`, and the no-orchestrator/gateway-import purity check from `task-2-1`. `TestSpecMutationFailsGate` shows the (a) admission assertions are not trivially green: a `wrong_directory` mutation of each spec template is demonstrably rejected by both gate replicas, and a `wrong_token` mutation flips the verdict for drafts. Without this, the assertion suite could silently pass against a permissive default. All 34 cases collect and pass locally on top of coder's task-2-1 (commit 15258a4) with `PYTHONPATH=shared:gateway:orchestrator`.
This comment has been minimized.
This comment has been minimized.
Autofix tracking{"Lint/Python": 1} |
This comment has been minimized.
This comment has been minimized.
![james-in-a-box[bot]](https://avatars.githubusercontent.com/u/8340608?s=60&v=4){kind=small}
There was a problem hiding this comment.
Contract Verification — PR #3141 (issue-3077 slice-2)
All slice-2 acceptance criteria verified against the implementation; all 34 tests in the new consistency suite pass locally.
Task-2-1: shared/egg_contracts/artifact_spec.py
| AC | Evidence |
|---|---|
resolve_artifact_path("plan-draft", "3077") → .egg-state/drafts/3077-plan.md; analysis-draft → -analysis.md; both identifier shapes resolve |
artifact_spec.py:126,138 + test_known_concrete_paths + _IDENTIFIERS parametrization (int=3077, str="issue-1557-v2") |
specs_for("plan", "task_planner") returns exactly the plan-draft row; spec_by_name round-trips every row |
test_specs_for_plan_draft_is_singleton + test_spec_by_name_round_trips |
| Rows immutable (frozen), one producer + ≥1 consumer | @dataclass(frozen=True, slots=True) (artifact_spec.py:41) + __post_init__ invariants (:75-92) + test_every_row_is_frozen + test_every_row_has_single_producer_and_nonempty_consumers |
| Module imports cleanly with no orchestrator/gateway deps | artifact_spec.py imports only __future__, collections.abc.Mapping, dataclasses — verified by TestSpecModuleIsPure.test_spec_module_imports_are_pure |
All five registered rows (analysis-draft, plan-draft, architect-output, architect-slices, risk-analyst-output) match the path conventions encoded by orchestrator/routes/pipelines.py::_get_draft_path (pipelines.py:5250-5263) and the architect / risk-analyst prompt f-strings (pipelines.py:14021, 14081, 14127, 14407). Note the deliberate mismatch between artifact name (hyphenated risk-analyst-output) and on-disk filename (underscored risk_analyst-output.json) is called out in the module docstring (artifact_spec.py:117-120) — that mirrors today's prompt prose, which Consistency (c) pins.
Task-2-2: shared/egg_contracts/tests/test_artifact_spec.py
| AC | Evidence |
|---|---|
Consistency (a): every spec path admitted by gateway/phase_filter.py AND shared/egg_restrictions/phase_patterns.py |
TestConsistencyA_PhaseFilterAdmits + TestConsistencyA_PhasePatternsAdmits, parametrized on both identifier shapes |
Consistency (b): _get_draft_path equals spec resolution for refine+plan, both shapes |
TestConsistencyB_GetDraftPathEquality.test_get_draft_path_equals_spec_resolution — 4 parametrized cases (int/str × refine/plan) |
| Consistency (c): prompt f-string literals match spec resolution | TestConsistencyC_PromptFStringLiterals checks both directions — test_every_agent_output_spec_appears_as_a_literal (spec → literal) and test_each_literal_resolves_via_some_spec (literal → spec) |
| Deliberate template mutation (parametrized) fails gate admission | TestSpecMutationFailsGate with @pytest.mark.parametrize("mutation", ["wrong_directory"]) + dedicated test_wrong_token_is_rejected_by_gateway_for_drafts — mutation regex (_mutate_template) asserts non-no-op before checking gate rejection, so the ratchet cannot be silently weakened |
| Every spec row exercised by a resolve/round-trip case | all_specs fixture unions every row via specs_for; test_spec_by_name_round_trips, test_resolve_artifact_path_matches_template, registry-shape tests all iterate the full union; test_expected_rows_are_registered pins the names against silent drops |
The mutation tests are the load-bearing piece per refine-risk-1: they prove the admission assertions aren't trivially green against a permissive default. The assert mutated_template != spec.path_template guard inside test_wrong_directory_is_rejected_by_gateway is a nice belt-and-braces against a future no-op mutation slipping through.
Local run: pytest shared/egg_contracts/tests/test_artifact_spec.py -v → 34 passed.
Top-level acceptance_criteria
The contract's top-level acceptance_criteria array is empty ([]); all AC live at the task level on task-2-1 / task-2-2. There are no ac-N identifiers for egg-contract verify-criterion. The task-level AC are verified above. (The orchestrator is also currently unreachable from this sandbox, so even a top-level criterion couldn't be marked via the CLI right now — noted for transparency.)
Out-of-scope diff: orchestrator/tests/test_consensus_wrapper.py
The PR also reformats slice-1's test_consensus_wrapper.py (16 line changes). All edits are cosmetic — function signature compressed to one line, multi-line string concatenations collapsed, single→double quote normalization on a docstring snippet. No semantic change. The fourth commit message ("Fix checks: apply automated formatting fixes") identifies this as collateral from an autoformatter. This is outside slice-2's declared files_affected (which is shared/egg_contracts/artifact_spec.py for task-2-1 and shared/egg_contracts/tests/test_artifact_spec.py for task-2-2), but as a pure formatting normalization on a sibling-slice test file it's not a contract violation worth blocking on.
BRC history
.egg-state/brc-history/3077-implement-slice-2.{json,md} are auto-persisted by egg-orchestrator (the third commit) — expected automation, no review concern.
Verdict
All task-2-1 and task-2-2 acceptance criteria are objectively met. The implementation matches the contract description, the helpers behave as specified, and the mandatory consistency suite covers the refine-risk-1 mitigation as designed.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Review
LGTM. This is a clean, well-scoped slice that adds a frozen artifact registry plus a comprehensive consistency suite pinning it against every existing path-knowledge replica. The spec accurately mirrors today's production paths, both identifier shapes work, and the mutation tests prove the gate-admission assertions are non-tautological. I verified the consistency-A checks manually against the live PhaseFilter and phase_file_verdict for all 5 spec rows × both identifier shapes — every path is admitted; every wrong_directory and (drafts-only) wrong_token mutation is rejected. CI is green.
orchestrator/tests/test_consensus_wrapper.py changes are pure formatting (long-line collapses, quote swaps) — no behavior change.
Non-blocking nits
1. Dead docstring reference (shared/egg_contracts/tests/test_artifact_spec.py:113). The docstring of _candidate_producer_roles_for_phase claims the safety net is test_specs_for_round_trips_every_row:
the failure surfaces via :func:`test_specs_for_round_trips_every_row`,
which compares the union of ``specs_for(...)`` against
:func:`spec_by_name` on every registered row.
No such test exists in this file. The actual safety net is test_expected_rows_are_registered. Either drop the reference or rename the existing test to match.
2. Fixture vs. module helper drift risk (test_artifact_spec.py:88-105). The all_specs fixture re-derives the registry by iterating specs_for(phase, role) over a hardcoded role roster (_candidate_producer_roles_for_phase). The module already exposes all_specs() (the function, not the fixture) for exactly this purpose.
The hand-rolled iteration creates a quiet drift trap: if a future row adds a producer role outside the hardcoded roster (e.g., a new refine-phase producer), the fixture silently skips that row, and every consistency-A / consistency-B / mutation test that consumes the fixture skips it too. The test_expected_rows_are_registered pin doesn't rescue this — it asserts expected ⊆ registered (where registered is the fixture's view), so a new row that's not in expected and not in the role roster lands in neither set and never gets exercised.
Fix: have the fixture return artifact_spec.all_specs() directly. The "test specs_for too" angle can be a separate small test against specs_for("plan", "task_planner") (which you already have at line 234).
3. is OR == is redundant (test_artifact_spec.py:198).
assert spec_by_name(spec.name) is spec or spec_by_name(spec.name) == spec_BY_NAME stores the same instance so is is always True. The or == branch is dead. Pick one (is is the right one — frozen dataclasses with structural eq could pass == even for a re-built spec, defeating the round-trip intent).
4. Purity check has a blind spot (test_artifact_spec.py:589-608). TestSpecModuleIsPure walks vars(mod), which catches imported callables but misses pure side-effect statements like import orchestrator.routes.pipelines # for side effects — those land in sys.modules without binding a name. Today the module is clean so this is theoretical; a sys.modules-based check after importlib.reload(artifact_spec) would be more robust, but it's not worth the bookkeeping for a regression that nobody's about to introduce.
5. Fixture duplication / shadowing. TestConsistencyA_PhaseFilterAdmits.phase_filter (class fixture) and module-level _gateway_phase_filter both produce PhaseFilter(). Consolidate. The class fixture also shadows the imported phase_filter module name — confusing on read; pick a different fixture name.
6. risk-analyst-output name/path inconsistency (artifact_spec.py:170-178). The artifact name uses a hyphen (risk-analyst-output) but the filename uses underscore (risk_analyst-output.json). The comment at line 105-109 acknowledges this is intentional (the filename matches the role name risk_analyst, the artifact name follows the hyphenated convention). It's defensible but ergonomically rough — any future spec consumer that does string-munging to derive paths from names will trip on this. Worth flagging on the slice-3 PR to make sure the resolver always goes through spec_by_name(...).resolve_path(...) and never reconstructs the path from the name.
7. consumer_roles are not yet cross-checked against actual readers. The lists are documentation-only at this slice. That's fine for slice-2, but it means a wrong consumer list ships unchallenged. When slice-3 lands the propose-time validator and slice-4 wires the gateway read endpoint, please add a consistency test (or have the consumer audit happen as part of the slice-3 review).
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…us_wrapper.py The conflict was a dead local variable assignment (repo, _ = ...) that slice-1 removed via review feedback in PR #3140. Taking slice-1's version since the variables were unused.
Conflict Resolution SummaryResolved merge conflict with
DetailsThe single conflict was at
Adopted slice-1's deletion since the variables were dead and slice-1's change is strictly a cleanup that landed via reviewer feedback. Local No conflicts in the slice-2 contribution ( — Authored by egg |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review
LGTM — no slice-2 code change since my prior approval.
Delta inspection
Between my last review at 8d474463a and the current HEAD eb148cb0, the only PR-authored commit is the merge of origin/egg/issue-3077/slice-1. git diff 8d474463a..eb148cb0 -- shared/egg_contracts/ is empty — artifact_spec.py and test_artifact_spec.py are byte-identical to the version I previously approved.
The merge brought in slice-1's review-feedback commits:
| File | Change | Origin |
|---|---|---|
docs/architecture/coordination-state.md |
New "Slice landings" status table | slice-1 review feedback (PR #3140) |
orchestrator/consensus_wrapper.py |
Banner wording: "use the git show commands below" → "use the rendered git log / git show fallback commands in this prompt instead" |
slice-1 review feedback |
orchestrator/routes/event_prompt.py |
Empty-delta caution points at the re-review git log rather than absent git show |
slice-1 review feedback |
orchestrator/tests/test_compose_event_prompt.py |
Caution test asserts git log (matches the rendered command in the re-review path) |
slice-1 review feedback |
orchestrator/tests/test_consensus_wrapper.py |
Banner test now pins both SYNC_FAILURE_BANNERS+= append sites (unresolvable + merge-failed) |
slice-1 review feedback |
shared/prompts/REVIEWER-SYNC.md |
Delta-command row rewritten to describe the git log <sha>..<end_ref> re-review command + slice-2-4 forward references |
slice-1 review feedback |
These are all slice-1's PR (#3140) territory and were reviewed there; they are out of scope for slice-2 review.
Conflict resolution
The single semantic conflict at TestSyncOutcomesAndBanner._run_harness (orchestrator/tests/test_consensus_wrapper.py:1747) was resolved by adopting slice-1's deletion of repo, _ = (None, None) if not (tmp_path / "repo").exists() else (tmp_path / "repo", None). I confirmed the variables are dead — _build_harness is called with str(tmp_path / "repo") directly, and repo is never referenced in the function body. The resolved file is byte-identical to slice-1's version on those lines. Correct call.
CI
All real check suites green (Unit Tests, Integration Tests, Security Scan, lint, Docker). The single egg-reviewer-contract-verification failure is a workflow-internal issue (the status comment links to an Actions run that errored before posting a verification verdict), not a regression in the diff.
Prior non-blocking suggestions
My earlier <!-- has-suggestions --> nits on the slice-2 test file (dead test_specs_for_round_trips_every_row docstring reference; all_specs fixture re-deriving from a hardcoded role roster instead of returning artifact_spec.all_specs(); redundant is OR ==; consolidation of duplicate phase_filter fixtures) still stand — none of them were addressed in this revision, and that's fine: they're non-blocking and can ride a slice-3 follow-up. Flagging here just so they aren't lost in the re-review noise.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Apply egg-reviewer's non-blocking nits on shared/egg_contracts/tests/ test_artifact_spec.py: * Replace the bespoke `all_specs` fixture that re-derived rows via `specs_for(phase, role)` over a hardcoded role roster with a direct call to `artifact_spec.all_specs()` (aliased on import as `registered_specs`). Future rows whose producer role lies outside the refine + plan roster slice-2 originally covered are now exercised automatically. Removes the now-dead `_candidate_producer_roles_for_ phase` helper and the docstring reference to a non-existent `test_specs_for_round_trips_every_row` test. * Drop the redundant `or ==` branch in `TestResolutionRoundTrip.test_spec_by_name_round_trips` — `_BY_NAME` stores the same instance, so `is` is the load-bearing check (and the one that fails on a rebuilt-but-equal spec, which is the intended failure mode for a registry round-trip). * Consolidate the duplicate `PhaseFilter()` fixtures (`TestConsistencyA_PhaseFilterAdmits.phase_filter` class fixture and the module-level `_gateway_phase_filter`) onto a single module-scoped fixture, hoisted near the top alongside `all_specs`. Removes the shadowing of the imported `phase_filter` module name in the class. No behaviour change. All 34 tests in the consistency suite still pass locally.
Review feedback dispositionThanks for the thorough re-review. Per-item disposition for the 7 non-blocking nits from egg-reviewer's approve-with-suggestions:
Verification
— Authored by egg |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Contract Re-verification — PR #3141 (slice-2)
Incremental review of changes since 8d474463aa51ac2d1ca111b84d56f363115e4e7d. Two new commits on the PR branch (excluding egg/issue-3077/slice-1 merge-in):
eb148cb— Mergeorigin/egg/issue-3077/slice-1(conflict resolution inorchestrator/tests/test_consensus_wrapper.py: dead(repo, _ = …)assignment dropped per slice-1's review feedback). Slice-1 territory; no slice-2 contract surface touched.62b8929— Test-only cleanups inshared/egg_contracts/tests/test_artifact_spec.pyaddressing prior non-blocking nits.
Delta scope
Production code (shared/egg_contracts/artifact_spec.py) is unchanged since last review. Only the consistency test suite (shared/egg_contracts/tests/test_artifact_spec.py) was edited. Three refactors:
all_specsfixture now delegates toartifact_spec.all_specs()(aliasedregistered_specs) instead of re-deriving viaspecs_for(phase, role)over a hardcoded("refiner",)/("task_planner", "architect", "risk_analyst")roster. Removes the_candidate_producer_roles_for_phasehelper and the stale docstring reference. Strictly broader coverage — a future spec row whose producer sits outside the refine+plan roster will now be auto-exercised by every consistency / mutation test, which is exactly the refine-risk-1 ratchet posture the contract calls for.TestResolutionRoundTrip.test_spec_by_name_round_tripsdrops theor ==branch. Correct:_BY_NAMEstores the same instance built at module import, soisis the load-bearing check; a structurally-equal rebuild would==but notis, defeating the round-trip's "registry is source of truth" intent.- Duplicate
PhaseFilter()fixtures consolidated onto a single module-scoped_gateway_phase_filterhoisted to the top, removing the class-scopedphase_filterthat was also shadowing the imported module name.
Contract compliance
Contract task-2-2 acceptance criteria — re-confirmed:
- (a) Consistency assertions (a), (b), (c) all present and green:
TestConsistencyA_PhaseFilterAdmits(gateway) +TestConsistencyA_PhasePatternsAdmits(mirror) — both identifier shapes parametrizedTestConsistencyB_GetDraftPathEquality— refine + plan, both identifier shapesTestConsistencyC_PromptFStringLiterals— forward + reverse direction againstorchestrator/routes/pipelines.py
- A deliberate template mutation demonstrably fails the gate-admission assertion:
TestSpecMutationFailsGate(wrong_directory+wrong_token, both gate replicas). - Every spec row is exercised by a resolve/round-trip case (
TestResolutionRoundTrip,TestRegistryShape).
Local run: PYTHONPATH=shared:gateway:orchestrator pytest shared/egg_contracts/tests/test_artifact_spec.py → 34 passed.
Contract task-2-1 acceptance criteria (production module unchanged since last review): still satisfied — registered rows for analysis-draft, plan-draft, architect-output, architect-slices, risk-analyst-output; helpers resolve_artifact_path, specs_for, spec_by_name, all_specs exposed; module is pure (the TestSpecModuleIsPure no-import assertion still applies).
Verdict
No regressions to previously verified contract surface. The test-suite refactor strictly improves the refine-risk-1 ratchet by removing the hardcoded role roster as a drift surface. Slice-2 contract compliance preserved.
Note: the contract's top-level acceptance_criteria field is [] for issue-3077 — there are no ac-N items to mark via egg-contract verify-criterion. Per-task acceptance criteria are inline prose and aren't separately addressable from the CLI.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Re-review (62b8929)
LGTM. The single new commit since my prior approval is a focused fixture-cleanup pass that addresses the four actionable nits from 8d474463a cleanly — not just superficially. The other three items were either declined or deferred to slice-3/4 per my own original framing; all defensible. CI is green (Unit Tests, Integration Tests, Security Scan, Lint, Docker all SUCCESS).
Delta scope
git log eb148cb0..62b8929 --not origin/egg/issue-3077/slice-1 is the lone commit. Diff is 174 lines, entirely within shared/egg_contracts/tests/test_artifact_spec.py. No production code change, no spec/registry change.
Verification of each addressed item
-
Dead
test_specs_for_round_trips_every_rowdocstring reference — Fixed by deletion of the entire_candidate_producer_roles_for_phasehelper that carried it.grep test_specs_for_round_trips_every_rowis now empty in the file. ✓ -
all_specsfixture re-derives via hardcoded role roster (drift trap) — Fixed properly. The fixture now delegates toregistered_specs()(alias foregg_contracts.artifact_spec.all_specs(), imported on line 66). A future row with a producer outside the refine + plan roster will now flow through every consistency / mutation test automatically. The aliasing approach (from egg_contracts.artifact_spec import all_specs as registered_specs) sidesteps the name collision with the pytest fixture cleanly; alternative would have been renaming the fixture itself but that's a wider blast radius (every test signature would change). The choice is justifiable. ✓ -
Redundant
is OR ==— Fixed by pickingisalone (line 192). The new docstring (lines 184-190) explicitly calls out whyisis load-bearing — exactly the rationale I argued for. ✓ -
TestSpecModuleIsPureblind spot — Declined. The reviewer's own prior framing was "it's not worth the bookkeeping for a regression that nobody's about to introduce." The producer's disposition mirrors that. Accepted. -
Duplicate
PhaseFilter()fixtures +phase_filtershadowing — Fixed by removing the class-scopedTestConsistencyA_PhaseFilterAdmits.phase_filterfixture entirely. BothTestConsistencyA_PhaseFilterAdmits(line 256) and the twoTestSpecMutationFailsGatetests (lines 492, 537) now consume the single module-scoped_gateway_phase_filterfixture, which has been hoisted up next toall_specs(lines 104-109). No more name shadowing of the importedphase_filtermodule. ✓ -
risk-analyst-outputname/path inconsistency — Deferred to slice-3 review. My prior comment explicitly framed it as "Worth flagging on the slice-3 PR." Accepted. -
consumer_rolesnot cross-checked against actual readers — Deferred to slice-3/4 review. My prior comment explicitly framed it as a slice-3/4 review-time check. Accepted.
Spot checks
_gateway_phase_filter(underscore prefix) is consumed as a pytest fixture parameter — pytest fixture resolution matches by name regardless of leading underscore, so injection still works. Consistent with the original pre-cleanup name.- The
all_specs()module function returns_SPECS(an already-immutable tuple), so the fixture returns the same tuple identity across the module-scoped lifetime. No mutation risk. TestRegistryShape.test_expected_rows_are_registeredstill pins the five expected names against{spec.name for spec in all_specs}, so a silent drop of an existing spec row still fails loudly even though the fixture is now registry-wide.- The deleted
_candidate_producer_roles_for_phasehad no other consumers (grepped clean).
Out-of-scope diff
None on this revision — the prior orchestrator/tests/test_consensus_wrapper.py cosmetic edits and slice-1 merge are unchanged from eb148cb0.
— Authored by egg
This comment has been minimized.
This comment has been minimized.
| egg is addressing review feedback... |
Review feedback dispositionBoth reviews at the current HEAD (
The seven non-blocking nits from the earlier Verification
— Authored by egg |
|
egg feedback addressed. View run logs 14 previous review(s) hidden. |
…3152) * docs: mark #3077 slices 2-4 shipped in coordination-state doc Slices 2 (#3141), 3 (#3142), and 4 (#3143) have all merged since the coordination-state doc was last updated (it still read "as of slice-1"). Also updates the reviewer-worktree-sync section of concurrent-execution.md to reference egg-artifact as the served channel for spec-registered coordination artifacts (plan-draft, analysis-draft, architect-output) when worktree sync fails, replacing the shared-object-store git show fallback description that predates slice-4. * docs: mark #3077 slices 5-6 shipped in coordination-state All six slices of the #3077 epic have landed on main. Mark the slice-5 (prompt-sync ratchet) and slice-6 (fail-loud memory backend + Redis restart tests) rows Shipped, bump the section header to slice-6, and update the intro/closing prose so the design-target framing no longer contradicts an all-Shipped table. --------- Co-authored-by: jwbron <8340608+jwbron@users.noreply.github.com> Co-authored-by: egg-reviewer[bot] <261018737+egg-reviewer[bot]@users.noreply.github.com>
1 participant
shared/egg_contracts/artifact_spec.py with frozen ArtifactSpec rows and resolution helpers, plus MANDATORY in-slice consistency tests pinning every existing path hardcoding to the spec. No consumer behavior change yet.
Base PR: #3139
What's in this PR
Commits (3):
This slice
Artifact spec module: single declarative registry of coordination artifacts
Files affected:
shared/egg_contracts/artifact_spec.pyshared/egg_contracts/tests/test_artifact_spec.pyTasks (2) + acceptance criteria
shared/egg_contracts/artifact_spec.py(NEW): frozenArtifactSpecdataclass rows —name,path_templatekeyed on{identifier},phase,producer_role,consumer_roles— foranalysis-draft(refine),plan-draft(plan),architect-output,architect-slices, andrisk-analyst-output(plan), matching the paths currently encoded by_get_draft_path(orchestrator/routes/signals.py:1162-1166) and the agent-output conventions in.egg-state/agent-outputs/. Helpers:resolve_artifact_path(name, identifier),specs_for(phase, producer_role),spec_by_name(name). Pure Python, no orchestrator/gateway imports, no new config format (Genericize the SDLC pipeline into a declarative, config-driven workflow (custom phases + agents + per-phase slicing) #3017 consumes this later, decoupled).resolve_artifact_path("plan-draft", "3077")yields.egg-state/drafts/3077-plan.md;analysis-draftyields the-analysis.mdpath; both identifier shapes (issue number, pipeline id) resolve. -specs_for("plan", "task_planner")returns exactly the plan-draft row;spec_by_nameround-trips every row. - Rows are immutable (frozen) and each names one producer_role and at least one consumer role. - Module imports cleanly with no orchestrator/gateway deps.shared/egg_contracts/tests/test_artifact_spec.py(NEW): (a) every spec path (both identifier shapes) is admitted by the matchinggateway/phase_filter.pyphase gate AND theshared/egg_restrictions/phase_patterns.pymirror; (b)_get_draft_pathoutput equals spec resolution for refine and plan across both identifier shapes; (c) the prompt f-string literals inorchestrator/routes/pipelines.pythat name draft paths match spec resolution. Plus resolution/reverse-lookup round-trips for every registered row. These tests are the refine-risk-1 mitigation: any future drift in any replica fails CI instead of reproducing feat(orchestrator): validate producer's phase draft at the canonical path on propose (deterministic refine/plan gate input) #3016.Stack
issue-3077egg/issue-3077/slice-1