review: 2026-07-21 sprint -- dependencies, flag retirement, live-shape fixes (REVIEW-ONLY, do not merge)#61
Closed
thewrz wants to merge 20 commits into
Closed
Conversation
…eplace
The 2026-07-21 live smoke run rejected the date-clear batch: the server
type-validates replace values on /plannedStart and /plannedFinish as
strings and 422s on null ('Invalid value: must be string'). The same
builder feeds sync.py's date reconciliation, so a real --apply run that
clears a date hit the same failure.
op_planned_date(field, None) now emits {op:'remove', path:'/{field}'} --
the standard RFC-6902 unset, and remove ops are already live-confirmed
(index-based tag removal). Callers only queue a clear when the card
currently has the date, so remove-on-absent cannot occur.
The offline smoke double previously accepted any date value, which is
why tests passed while live failed. It now mirrors the live contract:
atomic batch validation that 422s on non-string replace values, and
remove support on the date paths. API-VALIDATION.md records the
2026-07-21 outcomes (set confirmed; clear-by-remove and blocked-clear
await the next smoke pass).
Closes #52
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
fix(agileplace): clear planned dates with RFC-6902 remove, not null replace
…ecks retired The second 2026-07-21 smoke run (after the issue #52 remove-op fix merged) passed the full sequence: step 6 read back isBlocked=False and both planned dates None, confirming the RFC-6902 remove clear and the blocked-state clear live. Every AgilePlace [live-check] item now has a recorded live outcome. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The io v2 public docs do not document the Dependencies feature's endpoints, and nothing gets built here against guessed shapes (issue #52 made the case). probe_dependencies.py answers the read-surface question against a real tenant: dump a card's GET keys (dependencies may already be embedded), verify plumbing via the documented connection/children control endpoint, then GET six candidate dependency paths and report status + body for each, with a devtools-capture fallback instruction when everything misses. Strictly read-only -- every request is a GET, and the test suite pins that invariant. Ships with the agreed design spec for the full blocked-by -> Dependencies feature (phases 0-2). Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three confirmed P2s from the local Codex (GPT-5.5, xhigh) pass: - The missing-env test deleted environment variables that env_config() immediately restored from .env on a configured checkout -- exactly the machine the probe is meant to run from. The test now points config.ENV_FILE at a nonexistent path. - Candidate classification conflated inconclusive with missing: a 204 (route exists, empty collection) was neither found nor missed, and 429/5xx counted toward the all-missing devtools fallback. Now 2xx = found, 404 = miss, anything else = inconclusive, and the definitive fallback only fires when every candidate returned a clean 404. - URLError/TimeoutError escaped probe_get as a traceback mid-diagnostic; transport failures now return status 0 and classify as inconclusive. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
First live probe run (production tenant) confirmed the undocumented read
surface: GET /io/card/{cardId}/dependency answers 200 with
{"dependencies":[]} -- singular path, matching the connection/children
pattern. Five sibling candidates 404'd cleanly against a passing control,
and the card GET embeds no dependency data. Populated entry shape and the
write endpoint/body remain [live-check] pending (Phase 0b).
Part of #57
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The UI-created EP-3A -> JPOWER1 dependency (mirroring GitHub edge #31 -> #68) read back as direction/cardId/timing/createdOn with timing 'finishToStart' -- and no dependency id, implying pair-addressed deletion. Write and delete request shapes remain [live-check] pending on a devtools capture. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DELETE /io/card/dependency with {cardIds:[dependent], dependsOnCardIds:
[blocker]} -- pair-addressed as the id-less read entries implied, plural
batch arrays in the card/connections style. Create shape remains the one
pending [live-check].
Part of #57
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…confirmed POST /io/card/dependency with the delete's pair body plus explicit timing:'finishToStart'. The POST response embeds a face projection with dependencyStats (native health counters). Read/create/delete are now all live-confirmed; only duplicate-create behavior remains fact-finding for the smoke round-trip. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dencies Phase 1 of issue #57, built entirely against the live-confirmed shapes (API-VALIDATION.md 'Dependencies API discovery'): - agileplace.py: card_dependencies (fail-closed read -- None means unknown, never empty), incoming_dependency_ids, and batch create/delete_dependencies using the captured pair bodies with timing finishToStart. - sync.py: sync_dependencies reconciles after the Blocked-flag pass, guarded by the same complete-snapshot check. Every GitHub edge is mirrored (Done blockers included -- the edge is structural; native dependencyStats display satisfaction), GitHub is authoritative only between two sync-managed cards, failed reads skip the card entirely (duplicate-create is unconfirmed live), plan-only ids never cross a read boundary, and timing is human-owned after creation (pair-matched reconciliation). - smoke.py: steps 11-12 add the dependency create/read/delete round-trip plus duplicate-create fact-finding (INFO); stale probe renumbered 13. - Offline doubles in test_run.py and test_smoke.py serve the confirmed endpoint shapes; the dry/apply parity harness normalizes the new depend/undepend mutations. The Blocked flag is untouched (phased decision -- spec Phase 2). Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
test_run._configure delenv'd GH_PROJECT_* but env_config() re-reads .env on every call and repopulates them, so on a configured checkout the offline world reached for the developer's REAL GitHub Project and failed with 'unexpected gh command: project item-list ...'. Reproduced by simulating a configured checkout, fixed by pointing config.ENV_FILE at a nonexistent path (the pattern test_config_env.py and test_smoke.py already use), verified green with and without a real .env present. Closes #59 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… double The 2026-07-21 smoke run (suffix 10a01e) retired the dependency round-trip live-checks: create/read/delete all behaved as coded, and the duplicate- create fact-find answered HTTP 409 Conflict 'Dependency already exists' (not idempotent). The offline FakeTenant now raises that exact rejection, which also exercises smoke's duplicate-probe SystemExit path offline, and API-VALIDATION.md records the outcomes -- no dependency [live-check] remains open. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…al review Three confirmed findings: - P1: an active issue blocked by a retired (NOT_PLANNED/DUPLICATE) issue with a URL-owned card lost its edge from the desired set (blockers resolved through active issues only) while the retired card stayed in the managed set -- the valid native dependency would be deleted as stale on every run. Blockers now resolve through _blocker_cards, which covers retired issues via retired_card_by_url, mirroring how the Blocked-flag path already treats retired issues as Done blockers. - P2: card_dependencies accepted entries without a usable direction or cardId and would treat a paginating response as a complete snapshot; acting on such a read re-creates an existing pair and the live 409 aborts the run. Reads now fail closed on any pageMeta member, unknown direction, or missing cardId. - P2: the main-level harnesses (test_sync_main, test_retired_issues, test_hierarchy_ownership) mocked every AgilePlace boundary except the new dependency read, so the suite made real DNS lookups silently converted to None -- the source of the flaky 3s-vs-58s suite times. All three now stub card_dependencies; the suite runs in ~0.3s flat. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ans now Issue #57 Phase 2, per maintainer decision after judging the native depends-on card icon on the live board: - sync.py sheds ALL Blocked-flag code: the flag-text mirror in step 4, the flag-clear during card retirement, and the now-dead stages.blocked_reason. The sync never writes /isBlocked or /blockReason; a human's flag (any reason text) is never touched. - clear_legacy_blocked_flags.py: one-shot throwaway batch-clear for the flags the old behavior left on the board. Signature-matched (fullmatch on 'Blocked by #N[, #M...]') so human-authored flags survive; dry-run by default; idempotent; to be DELETED with its tests once the board is clean. - ops_blocked/card_is_blocked/card_block_reason stay in agileplace.py as validated API surface (smoke still exercises the shape; the script is their remaining writer). - Tests pin the new contract: retirement is lane-only, no flag scrubbing, and no PATCH at all for a card already in its Done lane. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
clear_legacy_blocked_flags.py ran against the production board on 2026-07-21 (34 sync-authored flags cleared, 0 human flags touched) and was always destined for deletion afterward -- it lives on in git history. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex (gpt-5.6-sol, context-primed pass) flagged that README steps 1/4, the PATCH-batching paragraph, the module layout, sync.py's module and _retire_card docstrings, and smoke's step 5-6 docstring still described the retired flag mirror. All now state the Phase 2 contract: blocked-by mirrors as native dependencies, the Blocked flag is human-owned, and smoke's blocked round-trip validates API surface only. Part of #57 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The live create response is sparse: it carries the new card id but no version and no customId/laneId echo (confirmed during the first live migration, 2026-07-21). Indexing it as the card's snapshot queued redundant /customId and /laneId ops, which the issue-#8 stale-ops guard then read as a concurrent edit -- every fresh create+sync --apply aborted before any metadata landed and no state was saved. After an apply-mode create that returns an id, refetch the card once and index the fresh card as the snapshot. No redundant identity ops are queued, and the fresh snapshot's usable version lets the batched PATCH skip _card_with_version's own refetch, so net API calls are unchanged. A failed or mismatched refetch falls back to the sparse response rather than failing a run whose create already succeeded. Dry runs are untouched: planned-card snapshots flow exactly as before. The offline sync-run double now mirrors the sparse create response (id-only) and serves the persisted card via the single-card GET, pinning this path offline; API-VALIDATION.md records the new live fact. Closes #55 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adversarial review of the issue-#55 fix flagged that only the sync-run double was aligned with the live-validated create contract: the smoke FakeTenant still echoed the request body plus a default version -- a shape the live tenant never returns (version absent confirmed 2026-07-20/21; no customId/laneId echo confirmed 2026-07-21). Any future smoke code reading an echoed field would stay green offline and break live -- the same masking pattern that let #55 reach the first migration. The double now returns id-only, unconditionally, while still persisting the full card for the single-card GET to serve back. The create_returns_version knob is gone: a sparse default left it unused, and the versionless-create test now pins the default contract directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
fix(sync): index refetched card as snapshot after create
thewrz
marked this pull request as ready for review
July 21, 2026 17:55
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@API-VALIDATION.md`:
- Line 245: Update the fenced examples in API-VALIDATION.md, including the
DELETE and POST request blocks near the referenced sections, by adding http or
text as the language identifier on every opening fence. Ensure the additional
fenced example at the other reported location is updated as well, preserving all
example contents unchanged.
In `@docs/superpowers/specs/2026-07-21-blocked-by-dependencies-design.md`:
- Around line 3-6: Update the design summary to clarify that automatic
Blocked-flag writes are retired: identify the “stays” decision as Phase 1
history or state that the flag is now human-owned, consistent with Phase 2 and
the current implementation.
In `@smoke.py`:
- Around line 257-267: The duplicate dependency creation handling around
agileplace.create_dependencies must only accept a confirmed HTTP 409 response.
Inspect the SystemExit exception details and re-raise or otherwise fail
validation for 401, 5xx, transport, and any other unexpected failures; retain
the informational duplicate_detail result only for the conflict response.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1e8aedd5-06be-4279-85e1-f43d8719006e
📒 Files selected for processing (19)
.gitignoreAPI-VALIDATION.mdREADME.mdagileplace.pydocs/superpowers/specs/2026-07-21-blocked-by-dependencies-design.mdprobe_dependencies.pysmoke.pystages.pysync.pytests/test_agileplace.pytests/test_agileplace_dependencies.pytests/test_hierarchy_ownership.pytests/test_probe_dependencies.pytests/test_retired_issues.pytests/test_run.pytests/test_smoke.pytests/test_sync.pytests/test_sync_dependencies.pytests/test_sync_main.py
💤 Files with no reviewable changes (1)
- stages.py
…review Three CodeRabbit findings on the sprint review PR, all valid: - smoke's duplicate-create probe treated EVERY SystemExit as informational, so a 401/5xx/transport failure left the run green. Now that HTTP 409 is the confirmed live contract it is a hard pin: 409 passes, acceptance or any other failure FAILs the run (stricter than the suggested fix, which kept acceptance informational). Offline test models a 503 outage. - The spec's intro still said the Blocked flag 'stays' -- now states the Phase 1 -> Phase 2 history and the human-owned end state. - API-VALIDATION's two captured request examples gain MD040 language tags. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Review complete: CodeRabbit APPROVED after all 3 findings were fixed (a0cecc5, live on main) with threads replied and resolved. Closing without merge per this PR's review-only contract; deleting both review/2026-07-21-* snapshot branches. |
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
1 participant
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.
What this PR is
A review vehicle, not a merge candidate. Everything in this diff is already on
main(landed via fast-forward merges through 956e13b after live validation). Both branches are frozen snapshots: the base is main as of PR #51's merge (the last CodeRabbit-reviewed state), the head is main after the sprint. Flip this ready to trigger CodeRabbit's review of the whole sprint; when its threads are handled, close it — never merge it — and delete bothreview/2026-07-21-*branches.What the sprint contains
[live-check]items retired with recorded live outcomes./isBlocked//blockReason; a one-shot signature-matched cleanup cleared the 34 legacy flags (0 human flags touched) and was deleted after use..envleak into offline worlds (Offline whole-run tests leak the developer's real .env project config #59); unmocked dependency-read boundary (the flaky-suite-time source).Testing
Note: PR #56 (the #55 refetch-after-create fix) is intentionally NOT in this diff — it was never merged; it has been rebased onto main and awaits its own draft→ready flip.
🤖 Co-authored by Claude Fable 5.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation