feat(architecture): executable layer boundary contracts (Phase 3) - #464
Conversation
📝 WalkthroughWalkthroughThis change adds executable layer contracts for six services, versioned violation baselines, a standard-library checker, CLI commands, verification routing, and fail-closed tests for schema, coverage, classification, ratcheting, and policy integrity. ChangesLayer Ratchet Enforcement
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant check_layered_architecture.py
participant layered_architecture.py
participant Layer Contract
participant Layer Baseline
check_layered_architecture.py->>layered_architecture.py: Run layer ratchet
layered_architecture.py->>Layer Contract: Load and validate rules
layered_architecture.py->>Layer Baseline: Load approved violations and budgets
layered_architecture.py->>layered_architecture.py: Scan, classify, and compare
layered_architecture.py-->>check_layered_architecture.py: Return report and status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b68af56 to
ab84899
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
architecture/deltas/introduce-executable-architecture-contracts.json (1)
28-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider declaring the new surface in
affected_surfaces.This delta now records a third governed contract, the layer boundary ratchet.
affected_surfacesstill lists only the Phase 1 and Phase 2 surfaces, andsummarydoes not mention layer enforcement. Add a surface entry such aslayer-architecture-ratchetso the delta stays self-describing for later reviews and tooling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@architecture/deltas/introduce-executable-architecture-contracts.json` around lines 28 - 31, Add a layer-architecture-ratchet entry to the delta’s affected_surfaces list so the newly governed layer boundary contract is explicitly declared; keep the existing Phase 1 and Phase 2 surface entries unchanged.architecture/layer-contract.schema.json (1)
29-35: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument why
suffixstays in the enum although the checker always rejects it.
scripts/lib/layered_architecture.pyemitslayer_contract.suffix_ruleas an error for every suffix rule, andarchitecture/README.mdline 164 lists suffix rules as forbidden. The schema still advertisessuffixas a valid match kind. The gate fails closed, so this is not a correctness hole, but an author can write a schema-valid rule that can never pass. Add adescriptiononmatchthat statessuffixis accepted by shape and rejected by policy, so the split is visible in the schema itself.♻️ Proposed schema annotation
"match": { + "description": "Shape-level match kinds. 'suffix' is retained for detection only: scripts/lib/layered_architecture.py rejects every suffix rule with layer_contract.suffix_rule.", "enum": [ "exact", "prefix", "suffix" ] },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@architecture/layer-contract.schema.json` around lines 29 - 35, Add a description to the match property in the layer contract schema, documenting that suffix remains structurally accepted in the enum but is rejected by the layered architecture policy and checker. Preserve the existing exact, prefix, and suffix enum values without changing validation behavior.tests/test_layered_architecture.py (3)
727-731: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePin the exact required keys instead of counting them.
The docstring states that a truthiness heuristic lets a plausible stub pass. Lines 730-731 then use count thresholds, which have the same weakness: a stub with five unrelated required entries and six unrelated properties satisfies them. Pinning the exact key sets makes a removed constraint visible in the diff, which is the stated purpose of this section.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_layered_architecture.py` around lines 727 - 731, Replace the count-based assertions in the schema validation test with exact set comparisons for schema["required"] and schema["properties"] keys. Reuse the required_keys definition where appropriate and define or reference the expected property-key set so unrelated entries cannot satisfy the test; keep the additionalProperties assertion unchanged.
308-343: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated
architecture/copy into a fixture.
shutil.copytree(ROOT / "architecture", tmp_path / "architecture")is repeated in about fifteen tests, and most of them then read, mutate, and rewrite one JSON document. A fixture plus a small mutate helper removes the duplication and keeps each test focused on the mutation it asserts.♻️ Proposed helpers
`@pytest.fixture` def architecture_copy(tmp_path: Path) -> Path: shutil.copytree(ROOT / "architecture", tmp_path / "architecture") return tmp_path def mutate_document(root: Path, name: str, mutate) -> None: path = root / "architecture" / name document = json.loads(path.read_text(encoding="utf-8")) mutate(document) path.write_text(json.dumps(document, indent=2), encoding="utf-8")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_layered_architecture.py` around lines 308 - 343, Extract the repeated architecture directory setup from the tests into an architecture_copy fixture that copies ROOT / "architecture" under tmp_path and returns the test root. Add a mutate_document helper for loading, modifying, and rewriting JSON documents, then update the affected tests—including test_missing_document_fails_closed, test_valid_json_that_is_not_an_object_fails_closed, and test_corrupt_schema_file_fails_closed—to use these helpers while preserving their existing assertions.
569-575: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout to the subprocess call.
run_clistarts a full repository scan with no timeout. If the checker hangs, the pytest session hangs and reports nothing. A timeout converts that into a clearTimeoutExpiredfailure.The Ruff
S603and ast-grep command-injection hints on this call are false positives. The command list issys.executableplus a fixed repository path, and no shell is used.♻️ Proposed change
return subprocess.run( [sys.executable, str(ROOT / "scripts" / "dev" / "check_layered_architecture.py"), *args], capture_output=True, text=True, check=False, + timeout=300, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_layered_architecture.py` around lines 569 - 575, Update the subprocess.run call in run_cli to include an explicit timeout value for the repository scan, allowing hangs to raise subprocess.TimeoutExpired instead of blocking pytest indefinitely. Keep the existing fixed command list, capture_output, text, and check settings unchanged.Source: Linters/SAST tools
scripts/dev/check_layered_architecture.py (2)
105-106: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFail closed when the comparison did not run.
The exit status depends only on
error_count.LayerRatchetResultalso carriescompared. If a future loader path returnsreport=Nonewith no error-severity issue, this CLI exits 0 while the ratchet never compared anything. The rest of this change is fail-closed; the exit path should match.🛡️ Proposed hardening
- failed = result.error_count > 0 or (args.strict and result.warning_count > 0) + failed = ( + not result.compared + or result.error_count > 0 + or (args.strict and result.warning_count > 0) + ) return 1 if failed else 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/dev/check_layered_architecture.py` around lines 105 - 106, Update the exit-status logic near LayerRatchetResult handling to treat a result with compared=false as failed, alongside existing error and strict-warning conditions. Ensure the CLI returns success only when the comparison ran and no configured violations were found.
86-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the library serialization options instead of restating them.
render_layer_report_jsoninscripts/lib/layered_architecture.pyowns the byte-identical output contract (indent=2,ensure_ascii=False,sort_keys=True, trailing\n). Line 87 repeats those options for the result payload. If the library changes them, the CLI JSON drifts andtest_cli_reports_passing_json_statusstill passes.Export a shared
render_json(payload)helper from the library and call it from both places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/dev/check_layered_architecture.py` around lines 86 - 87, Add and export a shared render_json(payload) helper in layered_architecture.py that applies the library’s established JSON serialization options and trailing newline, then update render_layer_report_json and the CLI JSON branch to use it instead of calling json.dumps directly. Preserve byte-identical output and remove the duplicated serialization options from the CLI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openspec/changes/introduce-executable-architecture-contracts/tasks.md`:
- Around line 230-234: Update the verification record in the current task entry
to replace the system pytest command with the mandated .venv Python invocation
including -p no:cacheprovider, and add the required npx openspec validate
introduce-executable-architecture-contracts --strict result alongside the
existing checker and verification-plan results.
- Around line 192-234: 將 tasks.md 中 Phase 3 delivery notes
的英文敘述改寫為繁體中文，涵蓋「Declared deviation (3.1 / 3.2)」與「What landed」兩段及其驗證內容；保留
OpenSpec 必要標頭、識別符、工具名稱、檔案路徑、測試名稱、issue code、命令與數值原樣，不要修改技術內容或既有繁體中文。
In `@openspec/lifecycle-ledger.json`:
- Around line 1384-1389: Update the subject_commit field in the lifecycle-ledger
entry for the Phase 4 executable lifecycle contract to the commit containing the
updated Phase 3 tasks.md evidence, replacing the stale
6424a6d6074fa4adff1defe1b6cb7024277ff924 value. Keep the current_slice, task
counts, and last_verified fields unchanged.
In `@scripts/dev/check_layered_architecture.py`:
- Around line 67-83: Document the `--report-only` precedence in `_parse_args`
help text: it always emits JSON and ignores both `--format` and `--strict`,
including the fact that warnings do not affect its zero exit status. Keep the
existing report-only behavior unchanged.
In `@scripts/lib/layered_architecture.py`:
- Around line 358-370: Update the identity validation in the entry-processing
logic to validate the raw service, from, and to values before converting them to
strings. Reject missing, non-string, and empty values through
layer_baseline.entry_incomplete, then construct the identity only from the
validated fields so missing values cannot become the literal "None".
---
Nitpick comments:
In `@architecture/deltas/introduce-executable-architecture-contracts.json`:
- Around line 28-31: Add a layer-architecture-ratchet entry to the delta’s
affected_surfaces list so the newly governed layer boundary contract is
explicitly declared; keep the existing Phase 1 and Phase 2 surface entries
unchanged.
In `@architecture/layer-contract.schema.json`:
- Around line 29-35: Add a description to the match property in the layer
contract schema, documenting that suffix remains structurally accepted in the
enum but is rejected by the layered architecture policy and checker. Preserve
the existing exact, prefix, and suffix enum values without changing validation
behavior.
In `@scripts/dev/check_layered_architecture.py`:
- Around line 105-106: Update the exit-status logic near LayerRatchetResult
handling to treat a result with compared=false as failed, alongside existing
error and strict-warning conditions. Ensure the CLI returns success only when
the comparison ran and no configured violations were found.
- Around line 86-87: Add and export a shared render_json(payload) helper in
layered_architecture.py that applies the library’s established JSON
serialization options and trailing newline, then update render_layer_report_json
and the CLI JSON branch to use it instead of calling json.dumps directly.
Preserve byte-identical output and remove the duplicated serialization options
from the CLI.
In `@tests/test_layered_architecture.py`:
- Around line 727-731: Replace the count-based assertions in the schema
validation test with exact set comparisons for schema["required"] and
schema["properties"] keys. Reuse the required_keys definition where appropriate
and define or reference the expected property-key set so unrelated entries
cannot satisfy the test; keep the additionalProperties assertion unchanged.
- Around line 308-343: Extract the repeated architecture directory setup from
the tests into an architecture_copy fixture that copies ROOT / "architecture"
under tmp_path and returns the test root. Add a mutate_document helper for
loading, modifying, and rewriting JSON documents, then update the affected
tests—including test_missing_document_fails_closed,
test_valid_json_that_is_not_an_object_fails_closed, and
test_corrupt_schema_file_fails_closed—to use these helpers while preserving
their existing assertions.
- Around line 569-575: Update the subprocess.run call in run_cli to include an
explicit timeout value for the repository scan, allowing hangs to raise
subprocess.TimeoutExpired instead of blocking pytest indefinitely. Keep the
existing fixed command list, capture_output, text, and check settings unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 741fb73c-99d2-4035-8be0-57baea465af7
📒 Files selected for processing (15)
architecture/README.mdarchitecture/architecture-contract.jsonarchitecture/deltas/introduce-executable-architecture-contracts.jsonarchitecture/layer-baseline.jsonarchitecture/layer-baseline.schema.jsonarchitecture/layer-contract.jsonarchitecture/layer-contract.schema.jsonopenspec/changes/introduce-executable-architecture-contracts/design.mdopenspec/changes/introduce-executable-architecture-contracts/specs/executable-architecture-contracts/spec.mdopenspec/changes/introduce-executable-architecture-contracts/tasks.mdopenspec/lifecycle-ledger.jsonscripts/dev/check_layered_architecture.pyscripts/lib/layered_architecture.pyscripts/verification-manifest.jsontests/test_layered_architecture.py
There was a problem hiding this comment.
Pull request overview
This PR lands Phase 3 of the introduce-executable-architecture-contracts change: an executable, language-level layer-boundary gate (ARCH-LAYER-001). It classifies every statically scanned module across the six services into a layer, judges intra-service edge direction against an allowed-dependency matrix, and enforces a fail-closed ratchet against an approved baseline that grandfathers exactly two attributed violations. It reuses the Phase 2 module-graph extractors rather than adopting the task's named third-party tools (dependency-cruiser/import-linter), recording that deviation machine-readably. The checker fits into the existing root-contract verification/CI dispatch and the OpenSpec lifecycle.
Changes:
- Adds
scripts/lib/layered_architecture.py(loader/validator/scanner/ratchet) and CLIscripts/dev/check_layered_architecture.py, with contract + baseline JSON and their schemas underarchitecture/. - Adds an extensive fail-closed/adversarial test suite that also pins the policy surface (service set, layer sets, allowed matrix, languages, schema keys) as literals so loosening the contract is visible in review.
- Wires the new scripts into
scripts/verification-manifest.json, activatesARCH-LAYER-001, and updates OpenSpec artifacts (tasks 3.1–3.3 done, ledger 16→19/26, spec/design/README/delta).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/lib/layered_architecture.py | Core module: contract/baseline loading, layer-set/rule validation, scanning/classification, and the baseline ratchet. |
| scripts/dev/check_layered_architecture.py | CLI entry point with --strict/--report-only/--format and byte-identical output emission. |
| tests/test_layered_architecture.py | Canonical, fail-closed, adversarial, and pinned-literal tests. |
| architecture/layer-contract.json | Per-service module→layer rules, allowed matrix, and tooling_deviation. |
| architecture/layer-contract.schema.json | Draft-07 schema for the layer contract. |
| architecture/layer-baseline.json | Two grandfathered violations with attributed debt and zero-slack budgets. |
| architecture/layer-baseline.schema.json | Draft-07 schema for the layer baseline. |
| architecture/architecture-contract.json | Adds active invariant ARCH-LAYER-001. |
| architecture/deltas/introduce-executable-architecture-contracts.json | Declares the additive layer-boundary ratchet contract change. |
| architecture/README.md | Documents the layer ratchet, verify commands, finding table, and known deviations. |
| scripts/verification-manifest.json | Routes the two new scripts into the root-contracts path class and target. |
| openspec/lifecycle-ledger.json | Advances slice to Phase 4, updates completed 16→19, last_verified. |
| openspec/changes/.../tasks.md | Marks 3.1–3.3 done with delivery notes and known limits. |
| openspec/changes/.../specs/.../spec.md | Adds the layer-boundary executable scenario. |
| openspec/changes/.../design.md | Records the Phase 3 tooling deviation. |
Note: I found one minor, low-severity issue — the baseline entry_incomplete re-enforcement stringifies fields before validating, so it fails to catch a missing (null/absent) service/from/to field, unlike the sibling debt_missing check. This can't produce a wrong "pass" (schema corruption fails closed elsewhere), but the guard is inconsistent with its stated intent. Otherwise the change is well-structured, thoroughly tested, and internally consistent (ledger↔tasks count, service ids↔contract, schemas↔documents all verified). Given the scope (~2,000 new lines), that it modifies CI/governance enforcement infrastructure and the machine-truth lifecycle ledger, human review is warranted.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b68af56b09
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Close introduce-executable-architecture-contracts tasks 3.1-3.3 with a standard-library layer boundary ratchet instead of the named third-party tools, disclosed in architecture/layer-contract.json tooling_deviation. - architecture/layer-contract.json: per-service module-to-layer rules, per-language layer sets and an exhaustive allowed-dependency matrix - architecture/layer-baseline.json: two grandfathered violations with attributed debt, budgets forced equal to the grandfathered count - scripts/lib/layered_architecture.py + scripts/dev/check_layered_architecture.py - tests/test_layered_architecture.py: 70 tests, incl. pinned policy literals - ARCH-LAYER-001 active; verification-manifest routes both scripts to the root-contracts gate
ab84899 to
55dd715
Compare
monkey1sai-blip
left a comment
monkey1sai-blip
left a comment
There was a problem hiding this comment.
Approved by monkey1sai-blip (the reviewer account pinned by the repo's merge governance).
Submitted through scripts/blip_review.py — a scripted approval carrying the operator's authority, pinned to head 55dd7159038e516cdbf324fa47e587d986632f22. This is the mechanism the GitHub App cannot satisfy: an App's approving review does not count toward required_approving_review_count.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55dd715903
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if language == "python": | ||
| edges |= _python_module_edges(root, repo_root, files, diagnostics) | ||
| modules |= {_python_module_name(root, path) for path in files} |
There was a problem hiding this comment.
When a service has multiple source roots, each call to _python_module_edges receives only the files from the current root, so an import from one root into another is silently unresolved. This affects the current bim-streaming-server configuration, which has separate messaging and setup roots: a future statically resolvable import between those packages can cross a forbidden layer boundary while the ratchet reports no edge. Build the module index for the entire service before resolving imports across its roots.
Useful? React with 👍 / 👎.
| for issue in blocking: | ||
| print(f"[ERROR] {issue.code} {issue.path}: {issue.message}", file=sys.stderr) | ||
| return 1 | ||
| report = build_layer_report(repo_root, observed_config, contract) |
There was a problem hiding this comment.
When observed-graph.config.json gains a service that is absent from the layer contract, this report-only path never calls _validate_coverage; build_layer_report simply skips that service and the command exits successfully with a partial report. Because the documented report-only output is used to prepare the approved baseline, require coverage validation here and return nonzero for uncovered services or language mismatches before rendering.
Useful? React with 👍 / 👎.
| rendered = "\n".join(lines) + "\n" | ||
|
|
||
| _emit(rendered, args.output, repo_root) | ||
| failed = result.error_count > 0 or (args.strict and result.warning_count > 0) |
There was a problem hiding this comment.
When the ratchet has warnings but no errors and the CLI is run with --strict, result.status remains passed, so both the human and JSON output report success even though this later calculation returns exit code 1. CI logs or consumers that read the structured status rather than the process code can therefore record a strict failure as passing; compute the strict-aware status before rendering and use the same value for the output and exit decision.
Useful? React with 👍 / 👎.
…465) 9f4a470 was PR #464's pre-squash implementation commit. Squash-merging discarded it, so it is not an ancestor of main and the machine-truth gate fails with subject_not_ancestor, which would red the next PR. Rebind to c5b9089, the squash commit that carries Phase 3 and stays reachable from main. Co-authored-by: monkey1sai <xshiujj@gmail.com>
…ifecycle row subjects (#474) * fix(openspec): derive the squash-introduced watermark for discarded row subjects Squash-merging discards the pre-merge commit a lifecycle row's subject_commit was reconciled at, so the recorded SHA stops resolving and assertRowSubjectAncestor red-flagged the next unrelated PR (subject_unavailable / subject_not_ancestor) until a manual rebind PR landed - #464 repaired 6424a6d, #465 repaired 9f4a470 and declared the general fix as follow-up work. This implements it: when the recorded subject is unavailable or not reachable, the verifier derives the newest HEAD-history commit that introduced that binding into openspec/lifecycle-ledger.json - under squash that is the squash commit itself, because the row and the sources it reconciled land atomically - and uses it as the staleness watermark. Source edits after that commit still surface as source_changed_since_subject, and a binding with no derivable introduction commit fails closed with the original error codes. assertRowSubjectAncestor becomes resolveRowSubjectWatermark and returns the effective watermark. The CLI suite adds squash recovery plus staleness survival after recovery; the existing fail-closed cases are unchanged and still pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6 * fix(openspec): resolve squash watermarks per row and keep non-ancestor subjects fail-closed Review round (CodeRabbit + Codex connector): - the introduction lookup now proves the {id, subject_commit} pair is present in a candidate's committed ledger and absent in its parent's, and the watermark cache is keyed per row - two rows sharing one discarded SHA resolve to their own introduction commits, so source drift after the older introduction is still flagged - squash recovery runs only for subject_unavailable; a subject that still exists as a commit but is not a HEAD ancestor stays subject_not_ancestor, fail-closed - the residual limit - edits folded into the same squash unit, guarded by that PR's own pre-merge gate run (where the recorded subject still resolved) and by the live task-count comparison - is documented in the code instead of claimed solved - regression tests: a committed orphan-bound row stays fail-closed; a shared discarded subject with two introduction commits flags exactly the drifted row Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6 * fix(openspec): fail closed on ambiguous row-binding reintroductions A binding introduced, rebound away, and reintroduced has multiple introduction commits; picking the newest could hide source drift between the introductions (Codex P2 on 17170c7). All -S candidates are now scanned and more than one introduction fails closed with the original error. Regression: intro -> drift -> rebind away -> reintroduce ends subject_unavailable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6 * fix(openspec): restrict squash recovery to introductions landed at the trusted base A candidate PR could mint a row bound to a fake 40-hex subject and have the fallback bless its own introduction commit (Codex P1, reproduced with --base at the parent). The unique introduction must now be an ancestor of the trusted base commit, so recovery only accepts squash commits main already accepted; candidate-side introductions stay subject_unavailable. Regression covers the reviewer's repro shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ommit (#477) Follows the Phase 3 convention (its row binds c5b9089, the #464 squash): after #475 squashed, the pre-merge branch commit 7c5dd98 stopped being reachable from any fresh clone, which fails machine-truth test 25 (subject_unavailable - the no-base call path cannot run the #474 watermark derivation) on every subsequent PR. The row now binds e7bb0b9, the #475 squash that carries the final Phase 4 tasks.md, which is permanently reachable main history. Claude-Session: https://claude.ai/code/session_015QTVFY89rS2xRwRB2TpFP6 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
introduce-executable-architecture-contracts: executable language-level layer boundaries (ARCH-LAYER-001), closing tasks 3.1, 3.2 and 3.3.architecture/layer-contract.jsonassigns all 207 scanned modules across six services to a layer with ordered, first-match-wins rules, and declares per-language layer sets plus an exhaustive allowed-dependency matrix. A layer with noallowedrow is an error, never a permissive default.architecture/layer-baseline.jsongrandfathers exactly two real violations with attributed debt, under per-service budgets the gate forces to equal the grandfathered count.layer-contract.json/layer-baseline.jsonand their schemas, plus invariantARCH-LAYER-001. These are additive governance contracts — no product API, event, or runtime contract changes, and no existing contract is relaxed.scripts/verification-manifest.jsonroutes both new script paths into theroot-contractspath class and target, so a change touching only the checker now dispatches its own tests (task 3.3).Declared deviation (3.1 / 3.2). The task text named
dependency-cruiser(npm) andimport-linter(pip). Neither was adopted. The canonical root-contract CI job installs onlypytestandjsonschemaonwindows-latest, so import-linter would require editing.github/workflows/ci.yml;apps/kit-manager-webhas nopackage-lock.jsonto pin dependency-cruiser against; and neither tool guarantees the byte-identical Windows/Linux output this repository asserts for architecture artifacts. Enforcement is insteadscripts/lib/layered_architecture.py, a standard-library checker reusing the Phase 2 module-graph extractors and the same ratchet discipline — the same shape as the declared 2.1 GitNexus deviation. The deviation is recorded machine-readably inlayer-contract.jsonundertooling_deviationand asserted by a test, so it must be superseded rather than deleted.Three-layer adversarial verification (two rounds, refute-by-default). Round 1 planted real forbidden imports in all six services — every one was caught — then found five ways to reach a green suite over a real regression. Round 2 confirmed five repairs and broke two of them: editing a service's declared
layersto match a collapsed rule set, and widening one row of theallowedmatrix, both laundered a real violation with the whole suite green. Because a ratchet over observed state cannot detect widened policy, the fix is an independent pin —PINNED_SERVICE_LAYERS,PINNED_ALLOWED_MATRIX, pinned per-service languages, the pinned set of layered service ids, and pinned load-bearing schema keys now live as literals intests/test_layered_architecture.py. Loosening the contract now requires editing the test file in the same diff, which is what makes it visible in review. Both round-2 attacks were re-run after the repair and red the suite.Round 3 is the PR review itself — CodeRabbit, Copilot and the Codex connector — and every finding was adopted:
layer_baseline.entry_incompleteread its values throughstr(entry.get(...)), andstr(None)is a non-empty string, so the guard never fired on a missing ornullidentity field; twoallowedrows for the samefromlayer silently replaced each other (layer_contract.duplicate_allowed_row);--report-onlynow exits 1 when any source file is unreadable or unparseable, because a report built from a partial scan must not be committed as a baseline, and--helpnow states that--report-onlyignores--formatand--strict; the Phase 3 delivery notes were rewritten in Traditional Chinese to match the rest ofopenspec/; andopenspec/lifecycle-ledger.jsonrebindssubject_commitfrom the unreachable6424a6d(the pre-squash head of #462) to the commit that carries this implementation.AI Coding Governance
Behavior contract changed = yesis additive only: this PR addsarchitecture/layer-contract.json+layer-contract.schema.json,architecture/layer-baseline.json+layer-baseline.schema.json, and invariantARCH-LAYER-001. No product API, event, or runtime contract changes, and no existing contract is relaxed. The change delta records it aschange_type: additive.Frontend Verification
passeddocs/plans/design-system-reference.manifest.jsonDeploy Path Verification
scripts/lib/andscripts/dev/python -m pytest tests -q -p no:cacheproviderValidation
Run in the governed worktree
.worktrees/introduce-executable-architecture-contractson Windows (Python 3.12.7, pytest 8.2.2, jsonschema 4.25.1):python -m pytest tests -q -p no:cacheprovider— 373 passed in 72.41s (294 before this change, 79 new).python scripts/dev/check_layered_architecture.py --repo-root . --strict— PASSED; 207 scanned files, 6 layered services, 2 grandfathered violations, 0 errors, 0 warnings; exit 0.python scripts/dev/export_observed_architecture.py --repo-root . --strict— PASSED; 0 errors, 0 warnings (Phase 2 ratchet unaffected).python scripts/dev/validate_architecture_contract.py --repo-root . --strict— PASSED; 0 errors, 0 warnings.npx openspec validate introduce-executable-architecture-contracts --strict— passed.npx openspec validate --all --strict— 71 passed, 0 failed.node scripts/tests/test-verification-plan.mjs— 22/22; a change touching onlyscripts/lib/layered_architecture.pyselects theroot-contractstarget with the same command CI runs.node scripts/tests/test-openspec-machine-truth.mjs— 24/24.git diff --check— clean.Cross-platform byte identity, actually measured.
check_layered_architecture.py --report-onlyproduces MD59e94d7996dee11baff7aa1a38d3627c9(24423 bytes) on both Windows and Linux from the same tree. Phase 2 asserted this property by discipline plus same-OS repeat runs; this is the first time it has been measured across the two operating systems.Known Risks
layer-contract.jsonis review-enforced, not gate-enforced. The ratchet judges observed state against an approved baseline; it does not judge whether the policy was widened. The pinned literals intests/test_layered_architecture.pyare the only mechanical defence, and they work by forcing the loosening into the diff. Disclosed inarchitecture/README.md§「Phase 3 的已知偏離與界線」.from app.x import yinservices/kit-manager-apiproduces no edge and no diagnostic, because the Phase 2 extractor resolves only against scan-root-relative module ids; the relative form is caught. No such import exists in-tree today, so this is a live hole rather than live debt. Fixing it means changing Phase 2's_resolve_python_target, which is out of scope here.windows-latestandweb-viewer-sample/tsconfig.jsondoes not setforceConsistentCasingInFileNames, so such an import can work at runtime while being invisible to the gate.ARCH-GRAPH-001, and same-layer edges are always permitted, which is not evidence the design is healthy.architecture/observed-graph.config.jsonare not layered, so editingexclude_file_suffixescan remove a module from enforcement.violation_budgetsis now a hand-synced cross-check rather than an independent defence, and warnings do not flipstatus— only--strictandtest_canonical_repository_layer_ratchet_passestreat them as red.bim-streaming-serverandkit-manager-apihave exact-only rules, so every new Python module in them requires a contract edit. That cost is deliberate.apps/kit-manager-webundeclared-node debt held byobserved-baseline.jsonis untouched by this change.subject_commitwill again become unreachable frommainafter this PR lands, exactly as docs(architecture): close executable contract gates #462's did. That rot is a pre-existing property of binding the ledger to a pre-merge SHA under squash merges; it is not introduced here and is not fixed here.scripts/tests/reconcile-openspec-ledger.ps1 -Mode Reconcilecould not be run locally: it requires explicit OpenSpec/Node paths, SHA-256 values and base-controlled trusted roots that this session did not have.openspec validate --all --strict(71/71) and the ledger edit to19/26were verified instead; the Agent Governance required check is the authority on ledger consistency.