Skip to content

P271: close installed merges through order behavior and repository identity - #2031

Closed
trissim wants to merge 10 commits into
mainfrom
fix/P271-merge-order-repository
Closed

trissim wants to merge 10 commits into
mainfrom
fix/P271-merge-order-repository

Conversation

@trissim

@trissim trissim commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

P271

ProblemOrder owns closed_by_merge: the base returns an empty tuple, checked orders perform their existing merge/install close. TodoStore dispatches every held order through that capability; the type filter is deleted. PullRequests also gets each order's declared pull_queries so an unordered held finding does not crash batching by lacking an id.

RepositoryName is the frozen owner/name value. Its equality/hash own case-insensitive identity, and its existing wire form remains owner/name. Repo.of and FieldCodec's Todo/goal-action boundary decode it; Repo.holds compares the nominal values. SQL remains the existing TEXT column. The old _repo string parser and caller-side identity comparison are deleted. CLI display, GitHub arguments/cache keys and repository consumers use the value rather than splitting/restating it.

Source-first audit used the pinned refactor-audit Package parser over Core, tools and Toad; before evidence is P271-source-before.txt. Dynamic receiver binding is not an AST proof. Actual configured CommanderConfig loads/round-trips and all276 retained orders decode with the new repo field. No original state was changed by that read.

Current pushed head: 8bbbf9cae0b1b64dc067179a0d1a1883d4e1bd5f, draft #2031. First implementation pushed at19:33:50; subsequent checks finished after the19:40 deadline.

  • 15 targeted checks passed in10.56s: actual Git/install reconciliation with differently cased repositories, typed SQL/config/wire round trips, an unordered held finding, component-based equality independent of wire rendering, and rejection of canonical names that cannot be read back. The Todo-index fixture now preserves unrelated hunt indexes.
  • Current separate tools ratchet: exit0. Current separate src ratchet: timeout160, exit124; not a pass. Receipts: P271-wire-{src,tools}.log/.exit.
  • Historical production head e6ba5f9 passed separate src/tools ratchets against origin/main resolved to6b86a9dc1/550b56aa3 respectively. Those are not current-main acceptance. The four reviewed lint findings and literal part dispatch were fixed there, without suppressions or moving code elsewhere.
  • The intervening cfdea00 src call lost its exit receipt. Its complete report against65f58db93 contained increases in older order/standing/todo code and one goal-action import finding; the latter was repaired at this head. That uncertain call was not replayed.
  • The recorded grep prints0 (its normal no-match shell exit is1).
  • Actual configured CommanderConfig JSON round trip passes;276 retained live orders read as RepositoryName,259 match configured repos. This was read-only, not a fixture-bus claim.
  • Checker and baselines are unchanged. Historical implementation allocation trailers remain; source growth alone is not treated as a defect under the corrected ruling.

Required practice-installed-copy board and next-rotate proof is NOT complete. Board line counts from copy/main: not measured. No practice build was supplied; this task forbids venv creation/uv sync, and paired installation belongs to the installer. No unit-only or fixture-board acceptance claim; this draft is not ready to merge. No main merge, package installation or provider launch.

Assumed: equality normalization belongs only inside RepositoryName; its wire output uses canonical case, matching existing Todo storage. Repo-directory short-name selection is owned by the same value. P270 retains its separate merged-order flattening work. For integration, CreateProblemGoalAction.repo is RepositoryName too; its JSON schema/wire value remains a string. Any dispatch task's Python repo field must take the nominal value, not an unparsed string.

Agent-Comms-Order: c8538f36cf05

Copilot AI balanced review requested due to automatic review settings October 19, 2026 23:34

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim marked this pull request as draft October 19, 2026 23:34
@trissim
trissim force-pushed the fix/P271-merge-order-repository branch 2 times, most recently from 81c242d to 2f8061b Compare October 20, 2026 03:07
An unordered held finding has no PR id; let the same order family expose its actual PR queries instead of making PullRequests probe all kinds. Cover mixed-case install reconciliation, config/wire/SQL identity and findings.

Src-Budget: 84
Validate owner/name components through the declared grammar rather than redispatching strings. Keep typed API signatures within the existing lint limits.

Src-Budget: 6
Delete the redundant key projection and implicit string conversion. Equality, hashing and transport now consume the value's one case-normalized wire form.
Delete the duplicate separator refusal: missing, empty and extra separators already fail the value's component grammar. Keep GitHub error text at its existing boundary.
Equality and hashing derive the case-normalized component pair, not rendered text. Keep the query-index fixture from deleting the unrelated hunt index.

Src-Budget: 3
Case folding must not produce an overlength or undecodable stored name. Also repair the goal-action import finding exposed by current main.
The board already used Commander.orders, but rotate bypassed it through held_orders and could never close a newly installed merge on its next tick. Both now consume the existing Commander owner before selecting outstanding work.
@trissim
trissim force-pushed the fix/P271-merge-order-repository branch from b77e9a7 to 2fd1c6b Compare October 20, 2026 03:26
@trissim

trissim commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Current P271 checkpoint

Head: 2fd1c6bac0529cee4578901934e7883c148a9242, rebased onto captured origin/main ef4cc72da. Existing immutable Core order identity is unchanged. This comment supersedes earlier-head validation claims.

  • ProblemOrder.closed_by_merge() defaults to (); CheckedProblemOrder owns actual merge closure. TodoStore.reconcile_merges() dispatches every held order instead of filtering concrete order types.
  • RepositoryName owns validated owner/name components, case-insensitive equality/hash and the existing lowercase wire representation. Repo remains in main's repo.py, retaining its Git helper. Config lookup, Todo SQL/wire storage, GitHub queries, progress records and branch consumers take this value.
  • Preserved main's flat merged-order model, phase/accepted capabilities, and CommanderContext methods. The held-order view uses the context's existing reconciled Todo view; deleted module-level helpers and rotate were not restored.

Separate requested checks, each backgrounded under timeout 300

Base ef4cc72da, head 2fd1c6bac:

Root Exit Seconds Positive deltas Introduced findings
tools 0 10 {} {}
src 0 51 {} {}

Full outputs: /home/ts/.cache/agent-scratch/pi-toad-queue/p271-{tools,src}-pushed.json, with separate stderr and receipt files.

Actual application evidence and limits

Using existing installed dependencies, the real commander console entrypoint with explicit copied-root config returned 0 for both candidate and installed-main board reads. Each showed 81 rows, 3 merged/waiting-on-install rows, and identical problem/order-ID sets. Runs were sequential on the same captured copy, not independent runs from identical initial snapshots. Files: p271-board-explicit-copy.log and p271-board-main-same-copy.log in the same scratch owner directory.

The initial read-only bwrap attempt failed on /dev/null; after admitting devices it failed the existing private-registry ancestor guard. No guard was weakened. The successful run used explicit copied-root configuration without that sandbox. No native input was submitted. An additional read-only original/copy SQL comparison timed out and supplies no closure evidence. One CLI-source inspection returned UNKNOWN and was not retried.

No automated tests were run by this worker in this continuation. Next-service-tick merge closure remains unverified; obsolete rotate is deleted on main. Marking ready is readiness for review, not installed acceptance. The 23:25 ceiling was missed during the second rebase and final checks.

Assumed: merger owns fresh-environment installation rehearsal and installed service-path acceptance. Original live sessions and uncertain inputs are not replayed.

@trissim
trissim marked this pull request as ready for review October 20, 2026 03:32
@trissim

trissim commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Sent back by desktop merger mB: 6 conflict hunks in todos.py and test_todos.py against main a51e58b (imports, plus MergedProblemOrder.merged). main now carries OrderCheckFailedError, StallWatcherIncarnation and the AgentOrigin/ExternalTranscript imports; this branch adds RepositoryName/CommandTarget/TextStorage. Merge current main, keep both import sets where both are used, and give merged() one implementation (main's plus the repository identity). Rerun test_todos.py and both ratchets, and post the counts. Due 00:40.

@trissim

trissim commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Not merged: conflicts with main in repo.py and todos.py. Rebase.

@trissim

trissim commented Oct 20, 2026

Copy link
Copy Markdown
Contributor Author

Replaced by merged #2153, preserving current main merge behavior through declared order/repository owners. Initial batch: 200 passed/21 failed; later fixes and actual CLI/GitHub merge-closing proof are documented, not a clean-test claim. Final source/tools ratchets 0/0.

trissim added a commit that referenced this pull request Oct 20, 2026
P271/P283: repository identity and lasting problem progress (replaces #2031, #2055)
@trissim trissim closed this Oct 20, 2026
@trissim
trissim deleted the fix/P271-merge-order-repository branch October 20, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants