Ar/260713 improved agentic system - #109
Merged
Merged
Conversation
… public-inventory repair
3b552f5 added `abandoned` to FrontierState without regenerating dashboard/src/types/projection.{ts,schema.json}, so the pre-commit generated-projection check failed on every commit made after it -- and regenerating alone was not enough, because the sprint-graph frontier recipe had no variant for the new state and the dashboard typecheck then failed on it. The recipe takes the dormant tone, which is how the dashboard renders an abandoned thing everywhere else (grammar/Dot.tsx:77). Projection produced by the sanctioned generator: PYTHONPATH=mcp/src python scripts/sync-projection-types.py
The child-admission seal refused new and reopened leaves whenever any lifecycle cell left its default. That was a fair reading of "closeout has begun" while the integration vocabulary held only not-started/completed/blocked, but `checkpointed` joined it on 2026-09-12, and from then on every checkpoint landing silently sealed its own master: worktree_checkpoint_landing retires nothing and keeps the enclosure "for the work that continues", yet the master could never admit another leaf again. The guard's own test file had been deleted on 2026-08-24, so nothing caught it. Closeout is the only thing that closes a child set, so closeout is the only thing that decides now, and the module is gone with all four call sites. The integration cell stays in the refusal message as evidence, so an operator can still tell a master that closed out from one that merely landed a line. worktree_pause had the matching defect: a master holding no selection is already in the state a release produces -- it is the ordinary state of a master between landings -- and refusing it failed an intent the caller had just satisfied. It now reports atomic-series-already-vacant, paused, publishing nothing, with the same stop next step. An unreadable record and a record naming another master are still refused, because neither proves the master is inactive. mcp/tests/test_lifecycle_playthrough_end_to_end.py plays the whole lifecycle in order -- start, close out, integrate, checkpoint, pause, resume, and a leaf started after the landing -- because no single-boundary case could have seen either defect. It failed on its first run, at the pause.
The ledger is derived state and its rebuild is the authority, so a row the rebuild cannot resolve is a reported exclusion -- ledger_projection already publishes sourceRowsExcluded and sourceExcludedReasons for it. The sync transaction kept a second copy of that judgement: a descendant or merged memory.md had to carry every row its source did. Measured on this master: thirteen rows (eleven stale duplicates whose code commits map to a different memory commit, two naming memory commits that exist nowhere) were dropped from the ledger by its own closeouts, and the same rows then refused the integration gate and this one. The master whose closeouts had been correct became permanently unsyncable -- worktree_start for the next leaf refused with "sync-work-branch-invalid: resolved memory ledger dropped parent mapping(s)" while both sides were already current, so there was no merge to resolve and nothing the operator could do. This removes the sync half of the rule, so one answer applies everywhere and no surface restates the projection's judgement where it can drift from the reasons it classifies. A selection left mid-flight now says so and never as this call's own success: the reconciling result names the stuck master, when it was published, what it was doing and both exits, and a pass that succeeded beside a mid-flight record reports atomic-series-reconciling rather than synced. The shipped c-09 skill text and its eight generated copies are corrected in the same commit: they told agents that a memory merge validates that every exact parent ledger row survives, which was the third place the removed rule was stated. Proof: a descendant memory branch that dropped its source row syncs as already-current (mutation-proved against the removed rule), and a completed pass beside a mid-flight selection returns non-ok with the master, revision, time and both exits named. Unit 853/157, integration 219/33; ruff, ruff format, pyright and the layering gate clean. Landed through sanctioned direct execution: this defect blocks worktree_start for every leaf of this master, including the leaf that owns it (260913-LCA-L13), so no leaf enclosure could be created for the fix.
…ferred to integration
…ixture, and sync the ownership declaration
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Important Review skippedToo many files! This PR contains 416 files, which is 316 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (416)
You can disable this status message by setting the 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 |
![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: 67b21aeb66
ℹ️ 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".
Comment on lines
+321
to
+323
| return LedgerRow( | ||
| _full_name(memory_repo, row.code_commit), | ||
| _full_name(memory_repo, row.memory_commit), |
There was a problem hiding this comment.
When a historical ledger contains an abbreviated code SHA, this resolves it against the memory repository, where the code object normally does not exist. The abbreviation therefore survives into the selected mapping and gets written into the Code-Commit trailer; lookups using the full code HEAD then fail immediately, and the attribution can disappear entirely if the prefix later becomes ambiguous. Pass code_repo here and resolve the code and memory columns against their respective repositories.
Useful? React with 👍 / 👎.
Comment on lines
+291
to
+292
| if earlier.integrated_code_commit == later.integrated_code_commit: | ||
| return False |
There was a problem hiding this comment.
For two external-memory leaves that change only onboarding, both integrations legitimately reuse the same code commit while their memory commits remain ordered. Returning False solely because the code commits are equal means neither leaf precedes the other, so _ordered_atomic_landing_chain finds zero candidates and permanently refuses the master's closeout as an invalid chain. Equal code commits need to defer to the memory ancestry rather than making the pair incomparable.
Useful? React with 👍 / 👎.
Comment on lines
+121
to
+127
| updated = record_landed_integration( | ||
| contract, | ||
| landed=LandedIntegration( | ||
| strategy=PR_STRATEGY, | ||
| code_commit=commit, | ||
| memory_content_commit=args.landed_memory_content_commit.strip(), | ||
| ), |
There was a problem hiding this comment.
For an external-memory contract, the public tool defaults landed_memory_content_commit to an empty string and this path still records integration.status=completed. A retry with the actual memory commit is then ignored by the earlier already-recorded branch, while organizational completion requires an exact nonblank landed memory edge, leaving the contract terminally recorded but impossible to finalize correctly. Validate and prove the memory landing before writing the completed integration cell for external-memory contracts.
Useful? React with 👍 / 👎.
Comment on lines
+135
to
+140
| attributed = [ | ||
| match.group("value") | ||
| for line in message.splitlines() | ||
| if (match := _TRAILER_LINE.match(line.strip())) | ||
| ] | ||
| return attributed[-1] if attributed else None |
There was a problem hiding this comment.
This scans every message line even though the runtime attribution reader uses Git's trailer parser. If a historical commit body mentions Code-Commit: <sha> outside its final trailer block, the backfill treats it as already attributed and skips rewriting it, but %(trailers:key=Code-Commit) later returns no attribution, so the migrated mapping remains missing and repeated backfill plans continue to report it as complete.
Useful? React with 👍 / 👎.
Comment on lines
+923
to
+925
| "strategy": args.strategy, | ||
| "integrated_code_commit": commits.code, | ||
| "integrated_memory_content_commit": commits.memory_content, |
There was a problem hiding this comment.
The checkpoint result emits integrated_code_commit and integrated_memory_content_commit, while WorktreeCheckpointLandingResponse declares the camel-case fields integratedCodeCommit and integratedMemoryContentCommit with empty defaults. Because the envelope allows extras, serialization returns both the real snake-case hashes and empty camel-case values; clients following the declared response schema therefore observe that no commits landed. Emit the values using the model's declared field names.
Useful? React with 👍 / 👎.
| members: list[CloseoutProjectionMember] = Field( | ||
| default_factory=list, max_length=MAX_CLOSEOUT_CANDIDATES | ||
| ) | ||
| members: list[CloseoutProjectionMember] = Field(default_factory=list) |
There was a problem hiding this comment.
The response now accepts an unbounded members list, and the same change removes the MAX_CLOSEOUT_CANDIDATES refusal from graph validation. Since each of the at most 256 masters can contain an unbounded number of subtasks, one authored sprint can force the queue census and MCP response to materialize arbitrarily many candidates, increasing memory and response size without limit. Restore a hard admission bound or introduce bounded pagination.
AGENTS.md reference: AGENTS.md:L204-L206
Useful? React with 👍 / 👎.
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.
No description provided.