Conversation
…eckout (Tencent#823) Item 7. import --from-mr in 0.25.0 to 0.26.0-beta.3 wrote learnings/<date>-<title>.md, with source_mr in its frontmatter, into the learnings checkout and never committed it. Nothing published it. In single-repo mode it also kept `git worktree remove` from removing the checkout an older teamai left in .teamai/, so every pull and contribute stopped on CheckoutRefusedError. publishQueuedLearnings now takes the sync lock first, and under it, before listing the queue, queues every untracked file of exactly that shape (directly under learnings/, date name, source_mr), in the active namespace and with contribute's name, then deletes the original. It finds the one checkout this repo registers for the branch (git worktree list), so the shared checkout and the old .teamai/learnings-wt are both covered and another repository's never is. A file the branch or the queue already has, by source_mr or by content, is deleted instead, and the warning names what has it. A dry run touches nothing. Item 21. The branch side of that duplicate check was the checkout's own tracked files. In single-repo mode the checkout is often the old .teamai/learnings-wt, which nothing syncs any more, so a teammate's later import of the same MR was missed and the remnant went out as a duplicate. When there are remnants, the check now also fetches origin/teamai-learnings (best effort) and reads what origin has that the checkout's commit lacks. Item 20. pull --dry-run published the queue: publishQueuedLearnings honoured dryRun only for the remnants. It now stops after listing the queue, and pull prints "[dry-run] Would publish N queued learning(s)" instead of publishing or warning. Maintenance sweep. publishLearningsMaintenance staged all of learnings/, so a confidence write-back or a prune swept any uncommitted file into its commit. confidence write-back, prune and promote now return the files they wrote or removed, and only those are staged (a removed file git never tracked is left out, since naming it would fail the add). That exposed a second bug: simple-git lists a staged rename under `renamed`, not `staged`, so a `prune --archive` with nothing else to stage counted as nothing to commit and was never published. commitAndPushAt now counts renames. Tencent#814 follow-ups. drainCheckoutQueue is gone: the preAction migration moves a checkout's queue before contribute and import --from-mr. Retire-only now says "Retired <legacy> to <backup>: this project's data already lives in <partition>"; a linked worktree lands there too, so "Finished an interrupted migration" was wrong for it. config.yaml.*.tmp, the temp an interrupted config save leaves (Tencent#831), is ignored in the single-repo and project-scope .gitignore, and the single-repo self-heal adds it. Item 15. After a failed refresh, readableReportsWorktree called ensure without the reports lock, so it could create the checkout while a writer that had just taken the lock created it too. It now refreshes once more under the lock and throws the cause if that fails as well. Item 17. init replaced the team clone before saving the new config, so an init that stopped in between (an unknown --role, a busy queue lock) left the old team's config.yaml beside the new team's clone. Just before it clones another owner's repo, init now settles the old install as the final save would (queue set aside, indexes dropped) and moves its config.yaml to config.yaml.previous. A failed init then leaves no config, and commands ask for teamai init.
SaulMoro
force-pushed
the
fix-823-data-layout-followups
branch
from
September 25, 2026 19:28
14bd21b to
b018180
Compare
The PR description includes sufficient unit and representative real-CLI testing evidence. |
…ter a failed clone Maintenance now stages exactly the files it names: commitAndPushAt and the removed-file ls-files lookup pass --literal-pathspecs, so a learning named with [ or * no longer stages the stray files it matches as a pattern. init reads the config it set aside when the rerun finds none, so an init whose replacement clone failed no longer drops enabledAgents, disabledAgents, toolRoots and inheritUserScope on the next run.
The earlier literal-pathspec finding is resolved. The PR description includes sufficient representative real-CLI testing evidence. |
…lain rerun An HTTP install's learnings dir is no git checkout, so the removed-file ls-files lookup threw after a prune had already deleted the file. It now returns the same non-fatal failed publish commitAndPush gives. init without --agent keeps the carried enabledAgents and disabledAgents, so a rerun after a failed replacement clone no longer reactivates tools uninstall --agent excluded.
The previous config-preservation, literal-pathspec, and HTTP-maintenance findings are resolved. The PR description contains sufficient representative real-CLI testing evidence. |
…led push kept local, keep remnants while origin is unreachable
The earlier config-preservation, literal-pathspec, HTTP-maintenance, maintenance-retry, and stale-origin findings are otherwise resolved. The PR description includes sufficient representative real-CLI testing evidence. |
…arnings branch, never let one bad maintenance record or remnant block the rest
|
Findings
The previously reported findings are otherwise resolved. The PR description includes sufficient representative real-CLI testing evidence. |
…, keep a maintenance record a read failed on
|
Findings
All previously reported findings are resolved in the current diff. The PR description includes sufficient representative real-CLI testing evidence. |
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.
Summary
The learning an older
import --from-mr(0.25.0 to 0.26.0-beta.3) left uncommitted in the learnings checkout now reaches the team, and it no longer keeps a single-repo project's old.teamai/learnings-wtfrom being removed. Maintenance publishes only what it changed.pull --dry-runno longer publishes the queue, and the leftover's duplicate check sees what a teammate published since the old checkout last synced. This also includes #814's follow-ups and items 15, 17, 20 and 21.Type of Change
Causes:
import --from-mrin 0.25.0 to 0.26.0-beta.3 (feat(learnings): write learnings to the teamai-learnings branch #616 until fix(import): publish the learningimport --from-mrextracts (#823) #825) wrote the learning into the checkout and never committed it. Git will not remove a checkout with untracked files without--force, so in single-repo mode everypullandcontributestopped onCheckoutRefusedError.publishLearningsMaintenancestaged all oflearnings/. Confidence write-back of an inherited learning,prune --archiveand the promote mark create new files, sogit add -ualone was not enough.prune --archive(found here, also broken on main): the archive move stages as a rename, which simple-git does not list instaged. With nothing else staged it counted as "nothing to commit", and the archive never left the machine..teamai/learnings-wt, which nothing syncs any more, so a teammate's later import of the same MR was missed and the leftover went out as a duplicate.publishQueuedLearningshonoureddryRunonly for the leftover adoption, sopull --dry-runpublished the whole queue (confirmed with the real CLI below).ensurewithout the lock, so it could create the checkout while a writer that had just taken the lock created it too.initremoved the old clone and cloned the new team repo long before it saved the new config, so aninitthat stopped in between left team A'sconfig.yamlbeside team B's clone.Test Plan
npx tsc --noEmitpassesnpx vitest runpasses: 328 files, 5067 passed, 1 skippedNew tests, red on
origin/main(7c834ce) unless marked guard:Ablations: each part was reverted alone and its test failed at the expected assertion. That covers adoption, the sync-lock guard, the dry-run guard, dedup (and source_mr-only), each signature check, the registered-checkout lookup, the namespace, staging the exact list, rename counting, item 15, item 17, Q10, the three Q11 lists, item 20 (the publish gate, the
Would publishline, the warning gate), item 21 (the origin read, and the fetch alone), and for Q9 the preAction migration that now owns the linked-worktree queue.Updated tests:
writeBackConfidencereturns the paths it wrote, andexecutePrune/executePromotionreturn theirs, so their assertions check those paths now. Theimport-mr-learnings-checkoutcase for a linked worktree's old queue calledimportCmddirectly, so it bypassed the preAction migration that owns that job since Q9; it moved to the e2e above. Inreports-branch-readonly, the "lock is free" case passed only because its mocked refresh failed and fell through to the unlockedensure(item 15). Its mock now has the realstatus()shape.E2E run (
npm run build,--retry 0): the new files, plusself-mode-worktrees-808,import-mr-publish-823,data-layout-migration,learnings-sync-704-705,init-project-all,init-unattended,reports-writer-sync-561,members-legacy-roster,dashboard-unified,deleted-worktree-scope-810,pull-new-worktree-807,scope-isolation-issue85andmulti-project: 14 files, 80 passed. The other files that runpull --dry-run(e2e,managed-resources-uninstall,push-stale-worktree-812,push-sync-followups-823,role-scoped-agents): 5 files, 34 passed, 22 skipped. The full e2e suite was not run. After the review fixes:self-mode-worktrees-808,learnings-remnants-823,init-project-all,init-unattended,data-layout-migration,multi-project: 6 files, 40 passed. After review 2:self-mode-worktrees-808,learnings-remnants-823: 2 files, 32 passed. After review 3: those two pluslearnings-sync-704-705: 3 files, 33 passed. After review 4: the same 3 files, 33 passed. After review 5: the same 3 files, 33 passed.Before: single-repo
contributewith the remnant in the old checkout:⚠ ... still has teamai-learnings checked out, and git will not remove it,Saved locally, nothing on origin.After:
⚠ Queued 1 learning(s) an older teamai import --from-mr left unpublished: …,✔ Contributed, the old checkout is gone, and both learnings are onteamai-learnings.Real CLI, #823 item 7 step 5: before (origin/main 7c834ce) and after (this branch)
recall maintenance --prune --archivealone, git mode (before / after): origin keepslearnings/low-note.mdand the checkout keeps a stagedR learnings/low-note.md -> learnings/_archive/low-note.md/✔ Published maintenance changes to the learnings branch, origin haslearnings/_archive/low-note.md.Real CLI, #823 item 20:
pull --dry-runwith one queued learning, before (14bd21b) and afterReal CLI, #823 item 21: single-repo, leftover in the old checkout, a teammate already published its MR; before (14bd21b) and after
Real CLI, review fix for item 17:
initrerun after the replacement clone failed, before (b018180) and afterReal CLI, review 2:
initrerun with no--agentafter the replacement clone failed, before (c23302d) and afterReal CLI, review 3:
recall maintenance --prune --archivewith origin offline, thenpull; before (8523c0d) and afterReal CLI, review 4: origin without
teamai-learnings, a leftover in the checkout, thenpull; before (331d929) and afterReal CLI, review 5: origin deleted the learning the checkout still tracks, a leftover from the same MR, then
pull; before (4defc3e) and afterRelated Issues
Part of #823 (item 7 and #814 follow-ups; also items 15, 17, 20 and 21).
Notes for Reviewers
Door: two-way. Blast radius: learnings publishing and maintenance,
initreplacing a team repo, reports readers.source_mror content. For a record, it meanscommitAndPushreturnedpublished, oralready-present(nothing staged and nothing pastorigin/teamai-learnings), or a successful read found it malformed. Audit of every delete site in the new code: the covered-remnantrmnow checks origin's tree, not the checkout; the queued-remnantrmfollows asavedqueue write; the malformed-recordrmnow needs a successful read; the published-recordrmis gated on the two statuses above. When origin cannot be fetched or read, or a queue entry or record cannot be read, nothing is deleted. Any other untracked file in the checkout is left alone.publishQueuedLearningsnow takes the sync lock before it lists the queue (sync, then queue lock, the same order as the migration), so two commands cannot adopt one remnant twice.pullstill passesholdsSyncLock.config.yamlreads the lastconfig.yaml.previous[.<n>]for the settings a re-init carries forward (enabledAgents,disabledAgents,toolRoots,inheritUserScope), and only those. With no--agent, both agent lists are kept as they were; before, any re-init without--agentdropped them, which contradicted the documented durableuninstall --agentexclusion. With--agent, behavior is unchanged. Queue ownership and the settle still go by the live config.initHttpnever moves a config aside and is unchanged.origin/teamai-learningsonly when there is a leftover to check, so ordinary runs make no extra network call. When the fetch fails, the leftover stays where it is until a run can fetch (review 3). When origin answersls-remotewithout the branch (an offline first publish never pushed it), origin adds nothing and the leftover is queued (review 4); only a failedls-remotekeeps it. Review 5: the duplicate check reads the fetched tree itself (ls-treeplus onegit showper learning), so a copy the checkout still tracks after origin deleted or replaced it no longer counts. That costs one git call per learning on origin, only in a run that finds a leftover.pull --dry-rundoes not preview leftovers a real run would queue or delete, and it still takes the queue lock briefly to list the queue (both P3 from the local review). Item 16 (viz) is left for [bug] Follow-ups to the worktree fixes: import --from-mr never shares its learning, and push can still revert a teammate's update #823./code-reviewfound no P1. Fixed: the version range (the issue said 0.22),<date>-.mdnames, a removed never-tracked file failing the maintenance add, docs wording. Not changed: item 17 settles the old install before the clone, so a failed clone followed byinit Aagain leaves A's queue inpending-learnings.git-<A>(named in the warning, nothing lost); settling after the clone would bring back item 17's busy-lock case. A remnant with the samesource_mras a published learning is deleted even if its content differs, as decided for [bug] Follow-ups to the worktree fixes: import --from-mr never shares its learning, and push can still revert a teammate's update #823.teamai-maintenance/, one file per run). No gitignore change is needed there, and the record goes with the checkout and its changes. Every laterpublishLearningsMaintenance, and everypublishQueuedLearningsunder the sync lock (pull,contribute,import --from-mr), publishes all the records, including when the queue is empty. A record is dropped only onpublishedoralready-present. A removed file that an earlier attempt already committed is left out of thegit add, so the commit it made is still pushed. Remaining edge: if a refresh rebases an unpushed maintenance commit that conflicts with a teammate's change on origin, the refresh drops that commit, as it always has, so the teammate's version wins, and the record then clears asalready-present. Review 4: records are written throughwriteFileAtomic, and only*.jsonis read, so an interrupted write leaves a.tmpnobody reads; a record that does not parse is removed with⚠ Removed unreadable maintenance record <file> (...), naming the checkout to check withgit status, and the other records still publish.