fix(spec): exclude volatile PlanningLog from ContractDigest#10
Closed
mamonisme wants to merge 216 commits into
Closed
fix(spec): exclude volatile PlanningLog from ContractDigest#10mamonisme wants to merge 216 commits into
mamonisme wants to merge 216 commits into
Conversation
- consolidate 11 files down to clear ownership: AGENTS.md is canonical agent guide, CLAUDE.md points to it as must-read - fix schema: add blocked status, require size/risk_level/planning_log/ created/updated, add move_to field, require description on criteria - differentiate strict vs standard validation profiles (boundary_check) - add resume protocol to exec.md - replace heavy ReAct logging with lightweight per-phase summaries - add complete example spec in .ai/specs/examples/ - slim .ai/README.md and specs/README.md to remove duplication - rewrite README.md as clean project overview
Add all missing fields: touchpoint owners/links, constraints.approvals_required, info_sources, planning_log notes, acceptance_criteria validation/result.output/ result.notes, definition_of_done notes, deviations array, integration and custom criterion types, manual validation type, and documentation criterion.
- Python CLI (cli/trellis) with init, new, list, status, validate, approve, start, complete, fail, cancel commands - Submodule-first workspace: trellis init creates symlinks, copies templates, installs CLI to ~/.local/bin - Update docs for CLI usage and workspace architecture
- trellis exec: runs acceptance criteria commands, records pass/fail results back into the spec YAML - trellis audit: compares declared spec files against git diff to detect scope creep - trellis diff: shows git history and uncommitted changes for a spec - auto planning_log entries on all status transitions - self-eval honesty checks on complete (warns on rubber-stamp 10/10) - warn if completing without exec results - trim unused schema fields (constraints, info_sources) - add result/executed_at/result_output to acceptance_criteria schema
- config overlay: .ai/config.local.yaml merges on top of base config so submodule stays updatable without losing project customizations - trellis report: aggregate stats across all specs (status breakdown, monthly activity, self-eval distribution, exec pass rates, phase stats) - replace fragile regex YAML parsing with indent-aware parser that handles both acceptance_criteria and validation blocks, dod_id fields, escaped quotes in commands, and varying indentation levels - trellis init now creates starter config.local.yaml - bump version to 1.1.0
- trellis repo: ignore __pycache__, .DS_Store, .ai/logs/ - trellis init: appends Trellis entries to project .gitignore (logs dir ignored, specs committed for audit trail)
Replace the symlink-from-submodule init with direct file copying. trellis init now copies config, schemas, prompts, and templates directly into the project. Added install.sh for one-liner setup. Bumped to v1.2.0.
Acceptance criteria can now declare a `cwd` field (relative to workspace
root) so commands run in the correct submodule directory. Validates that
the path exists and doesn't escape the workspace root. Commands without
cwd still run from root (backward-compatible).
acceptance_criteria:
- id: ac1
cwd: api
command: "bundle exec rspec spec/..."
Updated spec schema and README.
approve now runs schema validation before promoting a spec. Invalid specs are rejected with actionable error messages. exec --resume (-r) skips criteria that already have result: "pass" recorded in the spec. Useful for iterative development — fix a failing criterion, re-run with -r, and only the pending/failed ones execute. Extracted validate_spec() helper so validation logic is reusable. Bumped to v1.3.0.
exec now parses the expected field against command output instead of only checking exit codes. Handles "0 failures" (catches N>0 in output), "no matches", "exit code N", "Syntax OK", and literal substring match. approve now rejects specs containing TODO placeholders in actionable fields (command, content_spec, description, file, list items). Specs can set task.context.cwd as a default working directory for all acceptance criteria. Individual criteria can still override. Reduces repetition in monorepo specs where all commands target the same submodule. Bumped to v1.3.0.
- New `trellis review` command: runs automated passes (spec_compliance, scope_drift), scaffolds review file with context, prints adversarial prompt to stdout for the agent to execute - Review file accumulates rounds — each `trellis review` appends a new numbered section. Prior rounds provide context for subsequent reviewers. - `trellis complete` now gates on review: reads latest review section, parses verdict, refuses to archive on fail. --force to override. - Review verdict (with review_rounds count) recorded in archived spec - New review.md prompt template with three attack vectors: regression hunt, convention violations, defect scan - Updated spec schema, config, exec.md, AGENTS.md, README.md
- CLAUDE.md: add review mode, spec management rule - AGENTS.md: add review mode, spec management rule, review.md in key paths - .ai/OPERATORS.md: add trellis review to CLI, lifecycle, verification workflow - .ai/README.md: add review step, review.md and reviews/ to directory structure - .ai/specs/README.md: add review step to workflow, review field to anatomy
Enforce structured review artifacts and the audited human-reviewed override path. Add the review gate smoke workflow and README badge, make the smoke harness portable in CI, and ignore local review artifacts.
The review scaffold now includes Regression Hunt, Convention Check, and Defect Scan headings. parse_review_file tracks which are empty, and cmd_complete gates on all three having content. This prevents agents from rubber-stamping reviews by skipping adversarial vectors.
Make the review pipeline config-driven with ordered built-in passes and Review Artifact v3.\n\nAlso harden acceptance execution with nested result resume handling, per-criterion timeout support, generic pass expectations, refreshed docs, and expanded smoke coverage.
Use grep-based ordering checks in the smoke harness when ripgrep is unavailable so the GitHub Actions runner matches local behavior.
Handle nested acceptance result blocks and self_eval totals in complete/report. Also count phase statuses from the phases block only and cover the regressions with smoke tests.
update archived spec yamls to reference scafld; minor readme/cli/test updates
- new `scafld harden <task-id>` command scaffolds HARDEN MODE prompt and appends a round to `harden_rounds`; `--mark-passed` closes it - optional schema fields `harden_status` and `harden_rounds` (both non-required, backwards-compatible for specs missing them) - anti-hallucination grounding contract: every question must cite `spec_gap:<field>`, `code:<file>:<line>`, or `archive:<task_id>` - `scafld approve` intentionally unchanged - harden is operator-driven and never gates approval - spec template emits `harden_status: "not_run"` for discoverability - `scafld report` surfaces harden adoption (no gating, non-punitive) - docs, AGENTS, OPERATORS, planning, lifecycle, and spec-schema updated to describe the phase as optional - smoke test + CI wire-up enforces the optional-by-design contract
Keep the deterministic-protocol tagline; the finish line is now finalize signing an ed25519 receipt that scafld verify re-checks in CI, not a self-graded complete.
…ipts A failed envelope encode previously exited 0 with corrupt stdout; encode errors now print on the stream and force a non-success exit. Canonical receipt serialization checks marshal errors instead of discarding them.
Receipt files write atomically; latest.json is written only after the receipt is anchored in the session ledger. Ledger appends take a flock on unix so concurrent scafld processes cannot drop each other's entries. The signer refuses group- or world-accessible private key files.
fileDigests spawned one git cat-file subprocess per tree file per snapshot and hashed CombinedOutput, letting stderr noise reach the digest. One --batch process now streams deduped blobs over a dedicated stdout pipe.
The mutation guard recomputes only the tree fingerprint through a new Snapshotter.TreeSHA port method instead of a second full snapshot. The reviewer evidence reuses the per-file facts the signed snapshot already hashed, and reads evidence bytes through one cat-file --batch process instead of one subprocess per file.
provider.go keeps selection and receipt-grade wiring; provider client implementations move to clients.go and shared invocation, dossier, and event helpers to invoke.go. The duplicated review/harden progressLabel and first helpers collapse into the providerinfo package.
Keep completion ledger-authoritative after blocking reviews: a later passing review may clear a failure only when build evidence or changed review-attempt fingerprints prove a repair happened. Also make status avoid stale projected complete commands and fail finalize before invoking a provider on empty review evidence.
Describe targeted validation as the standard build-gate default and reserve full-suite evidence for release gates, shared framework changes, or explicit operator requests.
…s, FilesImpacted, Invariants, RelatedDocs)
…ariants, RelatedDocs) on round-trip renderContext only emitted CWD and Packages; the parser had no Context handler at all, so the entire Context block was silently dropped on any Parse after a Render/Save. Downstream consumers (reviewscope, harden, finalize) read model.Context.* and lost this data with no error.
ContractDigest is documented as a stable digest of the task contract and explicitly excludes volatile projection fields. It previously included PlanningLog, whose entries carry per-event timestamps, so two byte-identical contracts differing only in a planning-log timestamp produced different digests. In reviewgate.staleReviewAuthority this causes a valid, unchanged review to be falsely flagged review_stale_after_spec, forcing spurious re-reviews and blocking 'scafld complete'. Remove PlanningLog from the digest struct. Added regression tests: - digest stays stable across a planning-log timestamp change - digest still changes for a real contract edit (objective text)
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
3 participants
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
ContractDigestis documented as a stable digest of the task contract and explicitly excludes volatile projection fields (e.g. updated timestamps). It previously includedPlanningLog, whose entries carry per-event timestamps, so two byte-identical contracts differing only in a planning-log timestamp produced different digests.In
reviewgate.staleReviewAuthoritythis causes a valid, unchanged review to be falsely flaggedreview_stale_after_spec, forcing spurious re-reviews and blockingscafld complete.Root cause
internal/core/spec/model.goContractDigestmarshals a struct literal that includesPlanningLog []PlanningEvent(withPlanningEvent.Timetimestamps) into the SHA-256 digest, contradicting the function's stated stability contract.Fix
Remove
PlanningLogfrom the digest struct (2 lines). The planning log remains persisted elsewhere; it simply no longer participates in the contract digest.Tests
TestContractDigestStableAcrossPlanningLogTimestamp— digest stays stable when only a planning-log timestamp changes (regression for this bug).TestContractDigestChangesWhenContractChanges— digest still changes for a real contract edit (objective text), so the staleness signal remains live.go test ./internal/core/spec/passes.Failing-test branch (reproduces the bug against current main before the fix):
fix/contract-digest-planninglog-volatility— rungo test ./internal/core/spec/ -run TestContractDigestStableAcrossPlanningLogTimestamp -von1941ca5^to see it fail, and on1941ca5to see it pass.