fix(usage): discard pre-upgrade usage and stop falling back past an unreadable config (#748) - #758
Merged
Merged
Conversation
This was referenced Sep 23, 2026
|
Findings
The PR description includes both a test plan and real-CLI E2E records, so no documentation finding is needed there. No earlier review findings were supplied to mark as resolved. |
SaulMoro
force-pushed
the
fix/748-review-followup
branch
from
September 23, 2026 16:15
83fed01 to
11d475a
Compare
|
Findings
Resolved
PR Description
|
|
Findings
Resolved
PR Description
|
SaulMoro
force-pushed
the
fix/748-review-followup
branch
from
September 23, 2026 17:35
18a3c7a to
471373d
Compare
Collaborator
Author
|
Re: the two P1s from the last review (#754 and #751 "reverted") Both are false positives. The PR never touched those files. The branch was based on The "reverts" appear only when the PR head's tree is compared with the newer base checkout (two-dot), not in the three-dot PR diff. The branch is now rebased onto Also in this push: |
|
Findings
Resolved
PR Description
|
…nreadable config (Tencent#748) Follow-up to Tencent#753, from its review. - resolveConfigForDir returns null when any project config was reported unreadable, even if a lower-priority one (a legacy .teamai/ behind a broken partition) loads: that one may name another team. - The user scope's usage.jsonl is the old shared file. Tencent#753 only emptied it on a machine's first user-scope init, so a machine that already had a user scope reported every project's pre-upgrade usage to it. The first access after the upgrade now discards what an earlier release left there and writes ~/.teamai/usage-per-scope. One process discards, under acquireLock; concurrent hooks wait for the marker, so none deletes what another recorded.
…ck wording to team hooks (Tencent#748) - "handles empty file" wrote no marker, so the discard removed the file and the read passed on a missing file. A first read now settles the file as the scope's own, and the test asserts the file survives. - The session-start pull still resolves its project on its own, so the "never falls back to a lower-priority config" rule is stated for team hooks and skill usage only (CHANGELOG, usage guide en/zh-CN).
…a rollback (Tencent#748) The usage-per-scope marker could not tell a pre-upgrade event from one an earlier release appends after a rollback, so a reinstall reported those to the user-scope team. The user scope now records in ~/.teamai/user-usage.jsonl, which no earlier release writes; ~/.teamai/usage.jsonl is removed, never read. Drops the marker, its lock and the bounded wait.
…he user scope (Tencent#748) Also names the user scope's own file where comments and the design diagram still described every scope's usage as <dataHome>/usage.jsonl.
…ble project config resolveDoctorContext falls back past an unreadable project config the way detection does, so doctor diagnoses the config it falls back to and says nothing about the broken one (Tencent#752).
…it on every access (Tencent#748) getUsagePath deleted ~/.teamai/usage.jsonl on every user-scope call, including each hook append and the read-only `teamai stats`. The user scope never reads that file, which is what keeps its events off the team; the delete added a side effect to a path getter and a failure path to guard.
SaulMoro
force-pushed
the
fix/748-review-followup
branch
from
September 24, 2026 06:12
471373d to
99485c4
Compare
|
Findings
Resolved
PR Description
|
Collaborator
Author
|
Rebased onto |
jeff-r2026
approved these changes
Sep 24, 2026
This was referenced Sep 24, 2026
jeff-r2026
pushed a commit
that referenced
this pull request
Sep 24, 2026
* fix(votes): keep votes with the scope they were cast in (#787) Every scope recorded into one ~/.teamai/votes/<user>.yaml, so a vote cast in one project (recall feedback, a recall search, a Stop whose push failed) was pushed to the team of whichever scope synced next: the leak usage.jsonl had before #758. Votes now live in the data home of the scope that resolves for the session: <dataHome>/votes/ for a project, ~/.teamai/user-votes/ for the user scope. The Stop hook uses the config the dispatcher resolved; the pull report, recall search, `recall feedback` and the vote view read only that scope's votes, and the CLI readers resolve it with resolveConfigForDir, so an unreadable project config falls back to no other scope: `recall feedback` exits 1 and the vote view names the broken file. The shared ~/.teamai/votes/ is never read. Its V2 `votes` map is the last merged remote snapshot of whichever team synced, not this scope's history, and seeding a scope from it would let `recall feedback --negative` push a decrement and merged timestamps derived from another team. The remote votes/<user>.yaml format is unchanged. * fix(votes): address pre-review findings (#787) - recall search: in a project whose config cannot be read, detection falls back to another scope; record no recalled count there, so the vote cannot reach that scope's team. Which scope the search itself uses stays #796's. - recall feedback: with no project config and an empty or invalid user config, name the file and the fix (requireInit's error) instead of "not set up here". - CHANGELOG: note the recall search case; the shared directory is never read or pushed "by this release" (an earlier release still pushes it). - Design doc: getUserVotesDir() is the exception to "getters unchanged". * fix(votes): address second pre-review round (#787) - recall feedback: name an unusable user config through throwMissingOrInvalid (now exported) instead of re-running requireInit, which loaded the config twice and printed its parse error twice. - recall search: a project detection that throws is treated like an unreadable config, so no recalled count lands in the fallback scope. - The recall-search test now asserts the search ran and that neither the shared directory nor the broken project's votes/ was written; it fails on origin/main too. - Design doc: re-wrap the edited paragraph. * fix(votes): record recall votes from a deleted cwd in the user scope (#787) The previous commit treated any throw from recall's project detection as an unreadable project, including a cwd that no longer exists. Such a cwd holds no project and resolves to the user scope everywhere else (resolveConfigForDir, detectTeam), so its recalled counts belong there. Tests pin that case and the single parse-error line for an invalid user config in recall feedback. * fix(votes): address CI review findings (#787) - getVotesDir: a historical project-scoped ~/.teamai/config.yaml with no projectRoot (schema-valid, not backfilled) made getDataHome throw, so recall, feedback and the Stop hook recorded no vote. It lives in ~/.teamai, as recall and viz already treat it, so its votes go to the user scope's user-votes/. - git-native-memory design doc: the local votes path is user-votes/. * fix(votes): address CI review (#787) - recall feedback --negative counts the upvotes the scope's own team already holds (its reports checkout's votes/<user>.yaml plus the deltas not yet pushed). A scope's file starts empty on upgrade, so a doc upvoted before it was rejected as not found, or as having no upvotes once a later recall counted it. The shared ~/.teamai/votes and other scopes' teams are never read. - The adoption judge (#723, merged meanwhile) recorded into and synced from the user scope's votes in every scope, which pushed the user scope's pending votes to the project's team. It uses the scope's votes like the Stop handler. - votes-scope tests: Stop transcripts prove adoption with a Read of the recalled file (#723), and the update.js mock keeps the real lock.
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.
Follow-up to #753 (for #748). Fixes the two blocking findings of its last Codex review, which landed after the merge.
Summary
Before this, a machine that already had a user scope reported every project's pre-upgrade usage to the user-scope team on its first pull after upgrading. A separate file also covers a rollback: an earlier release goes back to writing every project's usage to
~/.teamai/usage.jsonl, and a later reinstall still never reads it.Evidence
Real CLI, isolated HOME, local bare team repos. The earlier release is a build of
95cea46(before #753).init --scope user, skill left by the earlier release.teamai/of another teamrollback-skillnot pushed (a marker-based discard,4ae9abe, pushed it)~/.teamai/usage.jsonlleft by the earlier release;stats,track,pullRed → green tests:
The init-only discard and its two tests in
http-repo-integrationare removed: the user scope never reads the shared file, so that case is covered too (row 2). Three #753 doc lines are corrected:<repo>/.teamaiholds usage only for an install not yet migrated to a partition (CHANGELOG),sessions/stays machine-level in theteam-intelligence-platformdiagram, and the CHANGELOG no longer claimsteamai doctorreports an unreadable project config (it falls back past one the way detection does, #752).Test plan
npx tsc --noEmit,npx vitest run(4504 passed, 1 skipped),npm run test:e2e(229 passed, 26 skipped),npm run buildgitprovider, Claude: the five rows abovereport-timeoute2e (git / gitlab / github × Claude, Codex, CodeBuddy, OpenCode) passes, seeding the user scope's new fileMerge Danger
Door: two-way. The user scope's unreported usage recorded by
0.26.0-beta.1in~/.teamai/usage.jsonlis dropped on upgrade, along with what earlier releases left there; those events name no project. A rollback leaves~/.teamai/user-usage.jsonlunread by the earlier release and goes back to its own shared file.Blast Radius: telemetry
Not in this PR → #752 (the handlers that read their config from the process cwd landed in #769): the session-start pull resolves its project with
detectProjectConfig()(src/pull.ts), so behind a broken partition it still reports that project's unreported sessions to a legacy.teamai/team, or to the user scope with no project excluded. Events recorded after the partition broke are not affected; the CHANGELOG already scopes its claim to exclude this pull.