chore: deploy Unbound Force v0.17.0 scaffold - #109
yvonnedevlinrh wants to merge 3 commits into
Conversation
|
🤖 Finished Review · ✅ Success · Started 7:57 AM UTC · Completed 8:38 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $12.88 |
ReviewFindingsMedium
Low
Next steps:
Previous runReviewFindingsMedium
Low
Next steps:
Previous run (2)ReviewOutcome: REQUEST CHANGES This PR deploys the Unbound Force v0.17.0 scaffold ( The review identified 9 medium-severity and 5 low-severity findings across five review dimensions. The most impactful findings are:
FindingsMedium (9)1. Stale convention pack paths in AGENTS.md and Divisor agentsFile: AGENTS.md and 5 Divisor agent configs reference Remediation: Update 2. Stale paths in cobalt-crush-dev.mdFile: cobalt-crush-dev.md (loaded via CLAUDE.md Remediation: Update all path references to the new canonical locations. 3. Permission migration drops
|
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
82dd3f1 to
88e7f7a
Compare
|
🤖 Finished Review · ❌ Failure (validation failed after 2 iteration(s)) · Started 10:01 AM UTC · Completed 10:43 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high |
em-redhat
left a comment
em-redhat
left a comment
There was a problem hiding this comment.
PR Review: #109 — chore: deploy Unbound Force v0.17.0 scaffold
CI Status
| Check | Status | Classification |
|---|---|---|
| Build and Test | FAIL | PR-caused |
| dispatch / Review | FAIL | Unknown |
| dispatch / Route | PASS | N/A |
| dispatch / Harness dispatch | PASS | N/A |
Walkthrough
| Directory | Files | Summary | Focus |
|---|---|---|---|
.opencode/agents/ |
11 | v0.17.0 scaffold: remove hardcoded model: fields, migrate tools: → permission: syntax, update pack paths |
standard |
.opencode/commands/ |
16 | Update Speckit pipeline commands with feature.json support, timestamp branching, template override stack | standard |
.opencode/skills/always-on-guidance/ |
1 | Remove ## Critical Safety section; merge sub-headers flat; change NEVER → Never |
standard |
.opencode/skills/pre-flight/ |
1 | Major rewrite: add soft-gate mode, Phase 2a file-scope filter, Phase 4a/4b baseline + causality |
standard |
.opencode/skills/review-context/ |
1 | Update consumer command references | documentation |
.opencode/uf/packs/ |
12 | Version bump entries only (+1/-1) | standard |
.specify/ |
15 | New scaffold state files, scripts, templates, workflows | security |
.gitignore |
1 | Replace granular .uf/ entries with wildcard; add .specify/ state entries |
standard |
Linked Issues
| Issue | Title | Criteria |
|---|---|---|
| #100 | Deploy Unbound Force v0.17.0 scaffolding | 10/11 criteria covered |
| ✗ Criterion 11: Run CI checks — not satisfied |
Summary
This PR deploys the Unbound Force v0.17.0 scaffold (agents, commands, skills, shell scripts, .specify/ state). The scaffold changes are well-structured, but the PR has a CRITICAL blocking defect: the always-on-guidance/SKILL.md update breaks the embedded copy drift-detection test, and a human reviewer has raised an unresolved scope objection.
Findings
CRITICAL — Broken CI: Missing embedded skill sync
The always-on-guidance/SKILL.md update removes ## Critical Safety, flattens sub-headers, and changes NEVER → Never, but the embedded copy at internal/agentkit/content/skills/always-on-guidance/SKILL.md was not updated. This breaks TestSkillFiles_DriftDetection (byte-compares the two copies) and puts TestAlwaysOnGuidance_StructuralHardening at risk (asserts ## Critical Safety exists, NEVER force push uses RFC 2119, hivemind_find is first Tool Usage item, and six sub-headers exist). See inline comment.
MEDIUM — Issue #100 criterion 11 not satisfied
The linked issue requires: "Run the repository's existing validation and CI checks." The PR description acknowledges the test suite was skipped. The resulting CI failure is a direct consequence.
LOW — .gitignore duplicate entry
.uf/feedback/ appears at both line 33 (legacy) and implicitly under .uf/ wildcard. Cosmetic only.
LOW — set -euo pipefail → set -e regression
Shell scripts in .specify/scripts/bash/ drop -u (unset var protection) and -o pipefail (pipeline failure propagation). The practical risk is low given explicit error handling on critical paths, but the regression is undocumented.
LOW — Unresolved human review objection
yvonnedevlinrh raised a scope concern ("These changes do not belong in this repo") that has not been addressed by the author.
Verdict
REQUEST CHANGES
The PR must resolve the always-on-guidance/SKILL.md embedded copy drift before merge. Either sync internal/agentkit/content/skills/always-on-guidance/SKILL.md and update the structural hardening test assertions, or revert the scaffolded copy changes. The author should also respond to the scope objection from yvonnedevlinrh.
This review was generated by /uf.review-pr (AI-assisted).
|
|
||
| # Always-On Guidance | ||
|
|
||
| Rules that apply to every coding session. |
There was a problem hiding this comment.
[CRITICAL] Embedded copy not synced — CI failure
This file was substantially rewritten (removed ## Critical Safety, flattened sub-headers, reordered Tool Usage Discipline, changed NEVER → Never), but the byte-identical embedded copy at internal/agentkit/content/skills/always-on-guidance/SKILL.md was not updated.
This breaks two tests:
TestSkillFiles_DriftDetection— byte-compares scaffolded and embedded copiesTestAlwaysOnGuidance_StructuralHardening— asserts the embedded copy contains## Critical Safety,NEVER force push(RFC 2119),hivemind_findas first Tool Usage item, and six sub-headers
Fix required: Either:
- Sync
internal/agentkit/content/skills/always-on-guidance/SKILL.mdto match this new content AND update the structural hardening test assertions to match the new structure, OR - Revert these changes to preserve embedded copy parity
Note: The structural hardening test assertions are a gatekeeping value (DR-002). Updating them requires explicit human authorization per the project's gatekeeping value protection rules.
There was a problem hiding this comment.
The embedded copy is not scaffolded by the uf binary, for the moment I have reverted the scaffolded version to match up with the embedded one, as the reverse would require tests to be updated and is outside of the scope of this PR. A follow up ticket to address this out of date embedded version will be created
… copy The v0.17.0 scaffold deployment updated the scaffolded copy at .opencode/skills/always-on-guidance/SKILL.md but did not sync the embedded canonical copy at internal/agentkit/content/skills/. This caused TestSkillFiles_DriftDetection to fail, breaking CI. Sync by copying the embedded canonical version over the scaffolded version to restore parity and fix the CI failure. A follow-up issue will track updating the embedded copy to adopt the v0.17.0 scaffold structure changes. Addresses PR #109 review feedback from @em-redhat. Signed-off-by: yvonnedevlin <yvonnedevlin@users.noreply.github.com> Assisted-by: claude-opus-4-6
Remove redundant .dewey/ entry from the Environment section — it is already listed under Legacy tool directories. Addresses PR #109 review feedback from @fullsend-ai-review and @em-redhat. Signed-off-by: yvonnedevlin <yvonnedevlin@users.noreply.github.com> Assisted-by: claude-opus-4-6
|
🤖 Review · Commit: |
|
Addressing each finding: CRITICAL — Broken CI: Missing embedded skill sync MEDIUM — Issue #100 criterion 11 not satisfied LOW — LOW — LOW — Unresolved human review objection |
… copy The v0.17.0 scaffold deployment updated the scaffolded copy at .opencode/skills/always-on-guidance/SKILL.md but did not sync the embedded canonical copy at internal/agentkit/content/skills/. This caused TestSkillFiles_DriftDetection to fail, breaking CI. Sync by copying the embedded canonical version over the scaffolded version to restore parity and fix the CI failure. A follow-up issue will track updating the embedded copy to adopt the v0.17.0 scaffold structure changes. Addresses PR #109 review feedback from @em-redhat. Signed-off-by: yvonnedevlin <yvonnedevlin@users.noreply.github.com> Assisted-by: claude-opus-4-6
Remove redundant .dewey/ entry from the Environment section — it is already listed under Legacy tool directories. Addresses PR #109 review feedback from @fullsend-ai-review and @em-redhat. Signed-off-by: yvonnedevlin <yvonnedevlin@users.noreply.github.com> Assisted-by: claude-opus-4-6
d89bc11 to
2a95d74
Compare
|
Addressing fullsend feedback
Resolved by other fix:
Out of scope (upstream
An upstream issue will be filed in |
|
🤖 Finished Review · ✅ Success · Started 11:49 AM UTC · Completed 12:14 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $14.27 |
|
Risk Assessment: moderate (2/5) DetailsSignals are unchanged from prior assessment -- same 77 files, 3987 lines, config/agent scaffold only, zero security-sensitive paths or CI changes -- keeping score at 2 (moderate); mild Tier 2 regression churn on agent files is consistent with scaffold history and does not elevate risk. Previous runRisk Assessment: moderate (2/5) DetailsLarge scaffold deployment (77 files, 3987 lines) across config and agent files only; the volume is offset by the absence of security-sensitive content, CI changes, or dependency modifications, and the well-scoped issue with clear acceptance criteria keeps overall risk moderate. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
… copy The v0.17.0 scaffold deployment updated the scaffolded copy at .opencode/skills/always-on-guidance/SKILL.md but did not sync the embedded canonical copy at internal/agentkit/content/skills/. This caused TestSkillFiles_DriftDetection to fail, breaking CI. Sync by copying the embedded canonical version over the scaffolded version to restore parity and fix the CI failure. A follow-up issue will track updating the embedded copy to adopt the v0.17.0 scaffold structure changes. Addresses PR #109 review feedback from @em-redhat. Signed-off-by: yvonnedevlin <yvonnedevlin@users.noreply.github.com> Assisted-by: claude-opus-4-6
Remove redundant .dewey/ entry from the Environment section — it is already listed under Legacy tool directories. Addresses PR #109 review feedback from @fullsend-ai-review and @em-redhat. Signed-off-by: yvonnedevlin <yvonnedevlin@users.noreply.github.com> Assisted-by: claude-opus-4-6
2a95d74 to
77ca0ab
Compare
|
🤖 Finished Review · ✅ Success · Started 12:25 PM UTC · Completed 12:58 PM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $14.00 |
| fi | ||
|
|
||
| # Validate branch name | ||
| check_feature_branch "$CURRENT_BRANCH" "$HAS_GIT" || exit 1 |
There was a problem hiding this comment.
[medium] logic-error
Inconsistent branch validation guard. check-prerequisites.sh calls check_feature_branch unconditionally (line 115), while sibling scripts setup-plan.sh and setup-tasks.sh guard the same call with feature_json_matches_feature_dir. Commands routing through check-prerequisites.sh will reject valid feature directories set via SPECIFY_FEATURE_DIRECTORY or feature.json on non-matching branch names.
Suggested fix: Add the same feature_json_matches_feature_dir guard around the check_feature_branch call in check-prerequisites.sh.
| > | ||
| > Resolve the conflicts manually, then re-run | ||
| > /uf.finale." | ||
| > /finale." |
There was a problem hiding this comment.
[medium] api-contract-violation
Bare slash-command references across multiple files. uf.finale.md uses /finale at lines 668, 727, 734 instead of /uf.finale. uf.review-council.md uses /review-council at lines 528 and 596. uf.init.md uses /unleash and /cobalt-crush at line 624. pre-flight/SKILL.md line 17 references /unleash. These bare references will cause command-not-found errors.
Suggested fix: Update all shortened references to include the uf. prefix: /finale -> /uf.finale, /review-council -> /uf.review-council, /unleash -> /uf.unleash, /cobalt-crush -> /uf.cobalt-crush.
| edit: true | ||
| bash: false | ||
| webfetch: false | ||
| permission: |
There was a problem hiding this comment.
[medium] permission-expansion
divisor-envoy.md, divisor-herald.md, and divisor-scribe.md are missing edit: deny in their permission blocks. All other Divisor reviewer agents include edit: deny. Without it, these three agents default to allowing file edits.
Suggested fix: Add edit: deny to the permission blocks of all three files.
| 8. Return: SUCCESS (spec ready for planning) | ||
|
|
||
| 7. Write the specification to SPEC_FILE using the template structure, replacing placeholders with concrete details derived from the feature description (arguments) while preserving section order and headings. | ||
| 6. Write the specification to SPEC_FILE using the template structure, replacing placeholders with concrete details derived from the feature description (arguments) while preserving section order and headings. |
There was a problem hiding this comment.
[medium] step-numbering
Duplicate outer step 6. The Outline contains step 6 at line 115 and another step 6 at line 140, making the numbering 4->5->6->6->7.
Suggested fix: Renumber the second outer step 6 to step 7 and cascade subsequent steps.
| .specify/extensions.yml | ||
| .specify/extensions/ | ||
| .specify/feature.json | ||
| .specify/init-options.json |
There was a problem hiding this comment.
[medium] gitignore-conflict
.specify/init-options.json and .specify/integration.json are committed AND listed in .gitignore, creating contradictory tracked-but-ignored state.
Suggested fix: Either remove the entries from .gitignore or remove the committed files.
| mode: subagent | ||
| model: google-vertex-anthropic/claude-opus-4-6@default | ||
| temperature: 0.1 | ||
| tools: |
There was a problem hiding this comment.
[low] api-shape-pattern
Retains old boolean tools: schema while all nine divisor-* agents were migrated to the new permission: schema.
Suggested fix: Replace the tools: block with a permission: block.
| **Automatic Pre-Hook**: {extension} | ||
| Executing: `/{command}` | ||
| EXECUTE_COMMAND: {command} | ||
|
|
There was a problem hiding this comment.
[low] formatting
Trailing whitespace on empty line inside code fence.
Suggested fix: Remove trailing spaces.
| @@ -41,66 +70,50 @@ Given that feature description, do this: | |||
| - "Create a dashboard for analytics" → "analytics-dashboard" | |||
| - "Fix payment processing timeout bug" → "fix-payment-timeout" | |||
|
|
|||
There was a problem hiding this comment.
[low] intent-coherence
Branch-creation logic removed. Branch creation is now delegated to an optional before_specify hook. Users without the git extension will silently work on the wrong branch.
Suggested fix: Add a fallback warning when no hook runs.
| "here": true, | ||
| "integration": "opencode", | ||
| "script": "sh", | ||
| "speckit_version": "0.9.4.dev0" |
There was a problem hiding this comment.
[low] intent-coherence
Committed init-options.json records speckit_version: 0.9.4.dev0, a pre-release dev version. Issue #100 authorized v0.17.0 scaffold.
Suggested fix: Confirm 0.9.4.dev0 is the correct bundled version or replace with the stable release version.
| to the default branch: | ||
|
|
||
| ```bash | ||
| DEFAULT_BRANCH=<detected per Phase 4a: Baseline Establishment, "Detect the default branch" subsection> |
There was a problem hiding this comment.
[low] intent-coherence
Phase 2a branch-diff computation references Phase 4a for default branch detection, but Phase 4a runs after Phase 2a. Forward reference creates circular dependency in document sequencing.
Suggested fix: Move default branch detection description to Phase 1 or early Phase 2.
3 participants
Summary
.gitignorefor v0.17.0-managed installer and runtime state.Scope
No production Go code, tests, workflows, or unrelated generated extension files are included.
Verification
.gitignore.Closes #100