[worktree-20260606-091709] test(delete-e2e): align two stale assertions with current behavior#696
Merged
Merged
Conversation
added 7 commits
June 6, 2026 11:57
ticket_link() in ticket-lib-api.sh discarded a --dry-run argument and always wrote the LINK event via ticket-graph.py --link, emitting no preview. Parse the position-independent --dry-run flag, strip it from positional args, and when set delegate to ticket-link.sh's preview path (no write, [DRY RUN] output). The non-dry-run path is unchanged. Adds a RED test exercising the canonical dispatcher dry-run path (asserts exit 0, [DRY RUN] preview, zero LINK events). DSO-Bug: 3796-ccd3-863f-4d63
…local Clarify that Step 5 (code-reviewer dispatch) and Step 6 (record-review) run ONLY when dso.workflow=local. Under dso.workflow=ci-pr they are skipped (no-op): CI enforces the review gate on the session->main PR (cumulative llm-review), and local enforcement is already deferred. Mirrors REVIEW-WORKFLOW.md's enforcement.strategy=ci skip path (the legacy alias that dso.workflow=ci-pr superseded; see CLAUDE.md rule:no-bypass-review).
…est files check-test-isolation.sh scans the full content of each staged tests/**.py file; on large files (e.g. test_ticket_graph.py, ~2528 lines) the rule loop takes ~63s, exceeding the 60s pre-commit budget and killing legitimate commits with exit 124 even though the check itself passes (exit 0). Raise the isolation-check wrapper timeout to 120s in .pre-commit-config.yaml and the shipped example for headroom on large files under load. DSO-Bug: 3a36-470f-fe35-4dd6
add_dependency() in ticket_graph/_links.py wrote any relation string verbatim (e.g. 'blocked_by'), bypassing the canonical grammar the legacy ticket-link.sh path enforced. Add a CANONICAL_RELATIONS guard that raises ValueError for unknown relations before any disk write; ticket-graph.py --link maps ValueError to exit 1. Adds RED tests in test_ticket_graph.py (uses monkeypatch.setenv for isolation) and test-ticket-link.sh. DSO-Bug: 61b8-bb44-fb04-402c
…APSHOT After ticket-compact.sh bakes LINK events into a SNAPSHOT's compiled_state.deps[] and deletes the *-LINK.json files, `ticket unlink` failed with "no LINK event found" because _get_link_info/_is_duplicate_link (ticket-link.sh) and _is_active_link (ticket_graph/_links.py) only consulted *-LINK.json/*-UNLINK.json. Add a SNAPSHOT fallback that scans compiled_state.deps[] for the link_uuid (minus any post-compaction UNLINK cancellations). RED test added: link -> compact (--threshold=1) -> unlink must exit 0, write UNLINK, and drop the dep from show. DSO-Bug: f5a8-d74d-7593-4313
test-ticket-delete-e2e.sh had 4 failures from two stale tests (production code is correct; the test predated two intentional changes): - E2E-1: closures now require a verdict hash (closure gate added in a6e8925). The test closed story/epic without one. Added a _close_with_verdict helper computing a real HMAC via compute-verdict-hash.sh; fixes the cascading S1/epic/full-lifecycle assertions. - E2E-6: imported _outbound_handlers from the bridge/ module deleted in a3a3928 (epic 3a03 edge->level-triggered reconciler cutover) and asserted a removed delete_issue route. Rewritten as test_reconciler_routes_deleted_to_done, asserting the reconciler outbound_differ maps deleted->Done (and never emits a delete action). UPDATE testing-mode: no skip/xfail, no production-code changes. PASSED:21 FAILED:0. DSO-Bug: e459-b5ef-9c02-403b
DSO-Story: worktree-20260606-091709
WalkthroughThis PR enhances ticket linking with canonical relation validation and snapshot-based recovery, adds dry-run support to the ticket link wrapper, refines integration workflow gating, and expands E2E test coverage with verdict hash enforcement and reconciler routing validation. ChangesTicket Link System: Validation, Snapshot Fallback, and Dry-Run
Configuration and Version Updates
Workflow Gate for Integration Modes
E2E Test Refactoring for Verdict Hash and Reconciler Routing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…rom ci-pr prose test-legacy-cleanup-grep.sh forbids legacy config-key references in plugins/dso/skills/. The ci-pr review-gating note referenced the literal `enforcement.strategy=ci` (legacy alias) in explanatory prose. Replace with a generic "CI-enforcement skip path" pointer to REVIEW-WORKFLOW.md + rule:no-bypass-review. The canonical dso.workflow gate logic is unchanged.
…tegrate fix: drop legacy enforcement.strategy token from single-agent-integrate.md (unblocks #696)
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/dso/docs/examples/pre-commit-config.example.yaml (1)
55-60:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix stale timeout comment to match configured value.
The inline comment says
timeout: 30s, but the hook is now configured to 120s (nameandentry). Please align the comment to avoid operator confusion.Suggested patch
- # Test isolation check (timeout: 30s) — staged-only scan with file-type + # Test isolation check (timeout: 120s) — staged-only scan with file-type🤖 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 `@plugins/dso/docs/examples/pre-commit-config.example.yaml` around lines 55 - 60, The inline comment above the isolation-check hook is stale (mentions "timeout: 30s") and should be updated to match the configured 120s timeout; edit the comment to read "Test isolation check (timeout: 120s)" (the hook is identified by id: isolation-check and references name: "Test Isolation Check (120s timeout)" and entry: ./scripts/pre-commit-wrapper.sh isolation-check 120 ...), ensuring comments and configured values are consistent.plugins/dso/scripts/ticket-lib-api.sh (1)
2115-2158:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the optional relation when
--dry-runis present.After stripping
--dry-run, this wrapper still requires three positional args. That breaksticket link <id1> <id2> --dry-run, even thoughticket-link.shaccepts[<relation>]and defaults torelates_to.Suggested fix
- if [ $# -lt 3 ]; then + if [ $# -lt 2 ]; then echo "Usage: ticket link <id1> <id2> <relation>" >&2 return 1 fi @@ - local src_id="$1" tgt_id="$2" relation="$3" + local src_id="$1" tgt_id="$2" relation="${3:-relates_to}"🤖 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 `@plugins/dso/scripts/ticket-lib-api.sh` around lines 2115 - 2158, The wrapper currently requires three positional args after stripping --dry-run, causing "ticket link <id1> <id2> --dry-run" to fail; change the arg-count check to require at least 2 (ids) instead of 3, initialize relation from "$3" but allow it to be empty (e.g. relation="${3:-}"), and when invoking ticket-link.sh in the --dry-run branch pass the relation argument only if non-empty so the underlying ticket-link.sh can apply its default (relates_to); update the usage check and the dry-run invocation that uses TICKETS_TRACKER_DIR and bash "$_TICKETLIB_DIR/ticket-link.sh" to reflect this conditional inclusion while keeping resolution logic for src_id and tgt_id unchanged.
🤖 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 `@plugins/dso/scripts/ticket-link.sh`:
- Around line 130-143: The loops that scan '*-SNAPSHOT.json' must use the newest
readable snapshot first and stop after processing that one so you don't
resurrect older state; in both _is_duplicate_link (the loop using snap_path,
snap, compiled, dep_target, dep_uuid, dep_relation) and the similar loop in
_get_link_info, iterate the glob results newest-first (e.g. sort in reverse or
by mtime descending), attempt to open each file until you successfully read one,
then process only that snapshot's compiled.deps and break (don’t continue
scanning older files after a successful read); keep the existing JSON/OSError
continue behavior for unreadable files so you fall back to the next-most-recent
file.
In `@plugins/dso/skills/shared/prompts/single-agent-integrate.md`:
- Line 181: The assignment to DSO_WORKFLOW uses a relative path to run
.claude/scripts/dso read-config.sh which can fail if the current working
directory is not the repo root; update the command to reference the repo root
variable (ORCHESTRATOR_ROOT) when invoking the script so it becomes something
like invoking "${ORCHESTRATOR_ROOT}/.claude/scripts/dso read-config.sh" to
produce DSO_WORKFLOW while preserving the existing fallback behavior
(DSO_WORKFLOW=${DSO_WORKFLOW:-local}); adjust only the path used to run
read-config.sh in the DSO_WORKFLOW assignment.
In `@tests/acceptance/test-ticket-delete-e2e.sh`:
- Line 359: The mktemp invocation that creates py_script uses an inconsistent
quote boundary: move the ".py" suffix inside the quoted template string passed
to mktemp so the template is consistently quoted (update the mktemp call that
assigns py_script in tests/acceptance/test-ticket-delete-e2e.sh to include ".py"
inside the "${TMPDIR:-/tmp}/reconciler-routing-test-XXXXXX" string).
In `@tests/scripts/test-ticket-link.sh`:
- Around line 1319-1347: The test currently only covers the three-arg dry-run
form ("ticket link <id1> <id2> relates_to --dry-run"); add a second test
invocation that exercises the two-ID documented form ("ticket link <id1> <id2>
--dry-run") using the same setup (id1/id2 from TICKET_SCRIPT) and the same
assertions: capture exit code and stdout_out from cd "$repo" && bash
"$TICKET_SCRIPT" link "$id1" "$id2" --dry-run, assert exit_code is 0, assert
stdout_out contains "[DRY RUN]", and assert
_count_link_events("$tracker_dir","$id1") remains "0" (no LINK event written).
Ensure this new invocation uses the same local variables (id1, id2, stdout_out,
exit_code) and descriptive assertion messages to mirror the existing canonical
dry-run checks so the wrapper bug is exercised.
---
Outside diff comments:
In `@plugins/dso/docs/examples/pre-commit-config.example.yaml`:
- Around line 55-60: The inline comment above the isolation-check hook is stale
(mentions "timeout: 30s") and should be updated to match the configured 120s
timeout; edit the comment to read "Test isolation check (timeout: 120s)" (the
hook is identified by id: isolation-check and references name: "Test Isolation
Check (120s timeout)" and entry: ./scripts/pre-commit-wrapper.sh isolation-check
120 ...), ensuring comments and configured values are consistent.
In `@plugins/dso/scripts/ticket-lib-api.sh`:
- Around line 2115-2158: The wrapper currently requires three positional args
after stripping --dry-run, causing "ticket link <id1> <id2> --dry-run" to fail;
change the arg-count check to require at least 2 (ids) instead of 3, initialize
relation from "$3" but allow it to be empty (e.g. relation="${3:-}"), and when
invoking ticket-link.sh in the --dry-run branch pass the relation argument only
if non-empty so the underlying ticket-link.sh can apply its default
(relates_to); update the usage check and the dry-run invocation that uses
TICKETS_TRACKER_DIR and bash "$_TICKETLIB_DIR/ticket-link.sh" to reflect this
conditional inclusion while keeping resolution logic for src_id and tgt_id
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 600e9097-d006-4acb-a7ee-9fd26d2271f5
📒 Files selected for processing (10)
.pre-commit-config.yamlplugins/dso/.claude-plugin/plugin.jsonplugins/dso/docs/examples/pre-commit-config.example.yamlplugins/dso/scripts/ticket-lib-api.shplugins/dso/scripts/ticket-link.shplugins/dso/scripts/ticket_graph/_links.pyplugins/dso/skills/shared/prompts/single-agent-integrate.mdtests/acceptance/test-ticket-delete-e2e.shtests/scripts/test-ticket-link.shtests/scripts/test_ticket_graph.py
Comment on lines
+88
to
+105
| for snap_path in sorted(_glob.glob(os.path.join(ticket_dir, "*-SNAPSHOT.json"))): | ||
| try: | ||
| with open(snap_path, encoding="utf-8") as fh: | ||
| snap = json.load(fh) | ||
| except (OSError, json.JSONDecodeError): | ||
| continue | ||
| compiled = snap.get("data", {}).get("compiled_state", {}) | ||
| for dep in compiled.get("deps", []): | ||
| dep_target = dep.get("target_id", "") | ||
| dep_uuid = dep.get("link_uuid", "") | ||
| dep_relation = dep.get("relation", "") | ||
| if ( | ||
| dep_target == target_id | ||
| and dep_relation == relation | ||
| and dep_uuid | ||
| and dep_uuid not in cancelled_uuids | ||
| ): | ||
| return True |
There was a problem hiding this comment.
Treat only the newest readable SNAPSHOT as authoritative.
This fallback walks all *-SNAPSHOT.json files in ascending order and returns on the first matching dep. If a later compaction writes a newer snapshot that no longer contains this dependency, an older snapshot will still make _is_active_link() return True once the intervening UNLINK has been compacted away.
Suggested fix
- for snap_path in sorted(_glob.glob(os.path.join(ticket_dir, "*-SNAPSHOT.json"))):
+ for snap_path in sorted(
+ _glob.glob(os.path.join(ticket_dir, "*-SNAPSHOT.json")),
+ reverse=True,
+ ):
try:
with open(snap_path, encoding="utf-8") as fh:
snap = json.load(fh)
except (OSError, json.JSONDecodeError):
continue
compiled = snap.get("data", {}).get("compiled_state", {})
for dep in compiled.get("deps", []):
dep_target = dep.get("target_id", "")
dep_uuid = dep.get("link_uuid", "")
dep_relation = dep.get("relation", "")
if (
dep_target == target_id
and dep_relation == relation
and dep_uuid
and dep_uuid not in cancelled_uuids
):
return True
+ break
Comment on lines
+130
to
+143
| for snap_path in sorted(p.glob('*-SNAPSHOT.json')): | ||
| try: | ||
| with open(snap_path, encoding='utf-8') as fh: | ||
| snap = json.load(fh) | ||
| except (json.JSONDecodeError, OSError): | ||
| continue | ||
| compiled = snap.get('data', {}).get('compiled_state', {}) | ||
| for dep in compiled.get('deps', []): | ||
| dep_target = dep.get('target_id', '') | ||
| dep_uuid = dep.get('link_uuid', '') | ||
| dep_relation = dep.get('relation', '') | ||
| if dep_target == target_id and dep_relation == relation and dep_uuid and dep_uuid not in cancelled_uuids: | ||
| print('DUPLICATE') | ||
| sys.exit(0) |
There was a problem hiding this comment.
Use the latest readable SNAPSHOT in both fallback paths.
Both embedded Python loops keep scanning older *-SNAPSHOT.json files until they find a match. After a second compaction, that can resurrect a dependency that was removed by a newer snapshot, so _is_duplicate_link() reports a phantom duplicate and _get_link_info() returns a stale link_uuid.
Suggested fix pattern for both loops
-for snap_path in sorted(p.glob('*-SNAPSHOT.json')):
+for snap_path in sorted(p.glob('*-SNAPSHOT.json'), reverse=True):
try:
with open(snap_path, encoding='utf-8') as fh:
snap = json.load(fh)
except (json.JSONDecodeError, OSError):
continue
compiled = snap.get('data', {}).get('compiled_state', {})
for dep in compiled.get('deps', []):
...
...
+ breakAlso applies to: 314-327
🤖 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 `@plugins/dso/scripts/ticket-link.sh` around lines 130 - 143, The loops that
scan '*-SNAPSHOT.json' must use the newest readable snapshot first and stop
after processing that one so you don't resurrect older state; in both
_is_duplicate_link (the loop using snap_path, snap, compiled, dep_target,
dep_uuid, dep_relation) and the similar loop in _get_link_info, iterate the glob
results newest-first (e.g. sort in reverse or by mtime descending), attempt to
open each file until you successfully read one, then process only that
snapshot's compiled.deps and break (don’t continue scanning older files after a
successful read); keep the existing JSON/OSError continue behavior for
unreadable files so you fall back to the next-most-recent file.
| Read the canonical workflow knob before dispatching: | ||
|
|
||
| ```bash | ||
| DSO_WORKFLOW=$(.claude/scripts/dso read-config.sh dso.workflow 2>/dev/null); DSO_WORKFLOW=${DSO_WORKFLOW:-local} |
There was a problem hiding this comment.
Anchor read-config.sh to ORCHESTRATOR_ROOT to avoid CWD-dependent failures.
Line 181 uses .claude/scripts/dso ... as a relative path, which can fail when the orchestrator is not in repo root at that moment. Use the same absolute-root pattern used elsewhere in this protocol.
Suggested patch
-DSO_WORKFLOW=$(.claude/scripts/dso read-config.sh dso.workflow 2>/dev/null); DSO_WORKFLOW=${DSO_WORKFLOW:-local}
+DSO_WORKFLOW=$("$ORCHESTRATOR_ROOT/.claude/scripts/dso" read-config.sh dso.workflow 2>/dev/null); DSO_WORKFLOW=${DSO_WORKFLOW:-local}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| DSO_WORKFLOW=$(.claude/scripts/dso read-config.sh dso.workflow 2>/dev/null); DSO_WORKFLOW=${DSO_WORKFLOW:-local} | |
| DSO_WORKFLOW=$("$(git rev-parse --show-toplevel)/.claude/scripts/dso" read-config.sh dso.workflow 2>/dev/null); DSO_WORKFLOW=${DSO_WORKFLOW:-local} |
Suggested change
| DSO_WORKFLOW=$(.claude/scripts/dso read-config.sh dso.workflow 2>/dev/null); DSO_WORKFLOW=${DSO_WORKFLOW:-local} | |
| DSO_WORKFLOW=$("$REPO_ROOT/.claude/scripts/dso" read-config.sh dso.workflow 2>/dev/null); DSO_WORKFLOW=${DSO_WORKFLOW:-local} |
🤖 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 `@plugins/dso/skills/shared/prompts/single-agent-integrate.md` at line 181, The
assignment to DSO_WORKFLOW uses a relative path to run .claude/scripts/dso
read-config.sh which can fail if the current working directory is not the repo
root; update the command to reference the repo root variable (ORCHESTRATOR_ROOT)
when invoking the script so it becomes something like invoking
"${ORCHESTRATOR_ROOT}/.claude/scripts/dso read-config.sh" to produce
DSO_WORKFLOW while preserving the existing fallback behavior
(DSO_WORKFLOW=${DSO_WORKFLOW:-local}); adjust only the path used to run
read-config.sh in the DSO_WORKFLOW assignment.
| # Write the test script to a temp file (avoids heredoc-in-subshell issues) | ||
| local py_script exit_code py_output | ||
| py_script=$(mktemp "${TMPDIR:-/tmp}/bridge-routing-test-XXXXXX".py) | ||
| py_script=$(mktemp "${TMPDIR:-/tmp}/reconciler-routing-test-XXXXXX".py) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Minor: inconsistent quoting in mktemp template.
The .py suffix is outside the quotes, which works but is inconsistent. Recommend including it inside for clarity.
- py_script=$(mktemp "${TMPDIR:-/tmp}/reconciler-routing-test-XXXXXX".py)
+ py_script=$(mktemp "${TMPDIR:-/tmp}/reconciler-routing-test-XXXXXX.py")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| py_script=$(mktemp "${TMPDIR:-/tmp}/reconciler-routing-test-XXXXXX".py) | |
| py_script=$(mktemp "${TMPDIR:-/tmp}/reconciler-routing-test-XXXXXX.py") |
🤖 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/acceptance/test-ticket-delete-e2e.sh` at line 359, The mktemp
invocation that creates py_script uses an inconsistent quote boundary: move the
".py" suffix inside the quoted template string passed to mktemp so the template
is consistently quoted (update the mktemp call that assigns py_script in
tests/acceptance/test-ticket-delete-e2e.sh to include ".py" inside the
"${TMPDIR:-/tmp}/reconciler-routing-test-XXXXXX" string).
Comment on lines
+1319
to
+1347
| local id1 id2 | ||
| id1=$(cd "$repo" && bash "$TICKET_SCRIPT" create task "DryRun canonical source" 2>/dev/null | grep -o '[0-9a-f]\{4\}-[0-9a-f]\{4\}-[0-9a-f]\{4\}-[0-9a-f]\{4\}' | head -1) | ||
| id2=$(cd "$repo" && bash "$TICKET_SCRIPT" create task "DryRun canonical target" 2>/dev/null | grep -o '[0-9a-f]\{4\}-[0-9a-f]\{4\}-[0-9a-f]\{4\}-[0-9a-f]\{4\}' | head -1) | ||
|
|
||
| if [ -z "$id1" ] || [ -z "$id2" ]; then | ||
| assert_eq "canonical dry-run: tickets created" "non-empty" "empty" | ||
| assert_pass_if_clean "test_canonical_dispatcher_dry_run_no_link_event" | ||
| return | ||
| fi | ||
|
|
||
| # Run via the CANONICAL dispatcher ($TICKET_SCRIPT, NOT $TICKET_LINK_SCRIPT) | ||
| local exit_code=0 | ||
| local stdout_out | ||
| stdout_out=$(cd "$repo" && bash "$TICKET_SCRIPT" link "$id1" "$id2" relates_to --dry-run 2>/dev/null) || exit_code=$? | ||
|
|
||
| # Assert: exits 0 | ||
| assert_eq "canonical dry-run: exits 0" "0" "$exit_code" | ||
|
|
||
| # Assert: output contains [DRY RUN] preview | ||
| if [[ "$stdout_out" =~ \[DRY\ RUN\] ]]; then | ||
| assert_eq "canonical dry-run: output contains [DRY RUN] preview" "has-dry-run" "has-dry-run" | ||
| else | ||
| assert_eq "canonical dry-run: output contains [DRY RUN] preview" "has-dry-run" "missing: $stdout_out" | ||
| fi | ||
|
|
||
| # Assert: NO LINK event written to disk (this is the failing assertion before the fix) | ||
| local link_count | ||
| link_count=$(_count_link_events "$tracker_dir" "$id1") | ||
| assert_eq "canonical dry-run: no LINK event written for id1" "0" "$link_count" |
There was a problem hiding this comment.
Add the two-ID --dry-run case here as well.
This only covers ticket link <id1> <id2> relates_to --dry-run. The wrapper bug shows up on the documented two-ID form, ticket link <id1> <id2> --dry-run, because --dry-run is stripped before the arg-count check.
As per coding guidelines, "all code changes require corresponding tests that fail before (RED) and pass after (GREEN)" and "Tests must exercise observable behavior of new code paths."
🤖 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/scripts/test-ticket-link.sh` around lines 1319 - 1347, The test
currently only covers the three-arg dry-run form ("ticket link <id1> <id2>
relates_to --dry-run"); add a second test invocation that exercises the two-ID
documented form ("ticket link <id1> <id2> --dry-run") using the same setup
(id1/id2 from TICKET_SCRIPT) and the same assertions: capture exit code and
stdout_out from cd "$repo" && bash "$TICKET_SCRIPT" link "$id1" "$id2"
--dry-run, assert exit_code is 0, assert stdout_out contains "[DRY RUN]", and
assert _count_link_events("$tracker_dir","$id1") remains "0" (no LINK event
written). Ensure this new invocation uses the same local variables (id1, id2,
stdout_out, exit_code) and descriptive assertion messages to mirror the existing
canonical dry-run checks so the wrapper bug is exercised.
Source: Coding guidelines
|
DSO-Review-Cycle: 1 pr_number=696 commit_sha=b878e0bb041490ef1d9172292efc80d125eb7742 findings_hash=d2d1f3d0c303e3a5 tuples=[["plugins/dso/scripts/ticket-lib-api.sh", "", "correctness"], ["plugins/dso/scripts/ticket-link.sh", "", "correctness"], ["plugins/dso/scripts/ticket-link.sh", "", "correctness"], ["plugins/dso/scripts/ticket-link.sh", "", "design"], ["plugins/dso/scripts/ticket_graph/_links.py", "", "correctness"], ["plugins/dso/scripts/ticket_graph/_links.py", "", "correctness"], ["plugins/dso/skills/shared/prompts/single-agent-integrate.md", "", "correctness"], ["tests/acceptance/test-ticket-delete-e2e.sh", "", "correctness"], ["tests/acceptance/test-ticket-delete-e2e.sh", "", "maintainability"], ["tests/acceptance/test-ticket-delete-e2e.sh", "", "verification"], ["tests/scripts/test-ticket-link.sh", "", "verification"], ["tests/scripts/test-ticket-link.sh", "", "verification"]] |
DSO llm-review — finding 1/11[critical] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 2/11[critical] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 3/11[critical] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 4/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 5/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 6/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 7/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 8/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 9/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 10/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
DSO llm-review — finding 11/11[important] Posted by dso_ci_review.runner; resolve this comment when addressed. |
CI failed: The CI run failed due to a combination of blocking code quality findings flagged by the LLM review agent and transient network connectivity issues preventing container image pulls.OverviewThis build failure is composed of two distinct issues: the automated LLM review detected 11 actionable code quality findings, and a separate infrastructure failure occurred due to network timeouts when pulling required Docker images. FailuresLLM Review Blocking Findings (confidence: high)
Infrastructure Network Timeout (confidence: high)
Summary
Code Review ✅ ApprovedAligns stale E2E assertions, fixes ticket-link dry-run handling, and rejects non-canonical link relations. Increases isolation-check budget and gates local code review, with no outstanding issues identified. Tip Comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
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
2 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.
Commits
Auto-generated by merge-to-main-pr.sh from
git log --no-merges origin/main..HEAD.Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests