[huong8373tt-beep]

Summary

This is the second dependent PR for #1459, building on #1468.

• Read member settings from the machine config once and overlay them below project/workspace and self-scope installs.
• Let local scope values win over machine values, including nested workflow tool and allow maps.
• Keep machine-only values out of workspace/self config files when those configs are saved.
• Ignore invalid machine settings individually with a warning while preserving valid sibling settings.
• Preserve settings explicitly changed by a command such as recall enable/disable when the effective value came from the machine overlay.

Validation

All validation below was run against the current rebased head ec70d09d.

• npm run build - passed
• npm run typecheck - passed
• npm run lint - passed (0 warnings, 0 errors)
• Focused regression suite - 248 passed across 5 files:
  • pull-scope-isolation.test.ts - 57
  • member-settings-overlay.test.ts
  • config-not-initialized.test.ts
  • update.test.ts
  • workflow-run-controls.test.ts - 99
• The previously failing pull-scope-isolation regression now passes with a fresh config object between pulls; the mock was also updated for the current main exports.
• git diff --check - passed

Representative built-CLI run

After npm run build, I ran the built CLI against an isolated HOME, a fresh workspace, and a local bare Git remote. The run exercised:

teamai init https://git.example.test/team/validation.git --provider git --agent claude --scope workspace --force
teamai pull --silent
teamai recall status
teamai recall disable

The run exited successfully and printed REAL_CLI_OK. It verified that:

• the workspace initialized and pulled successfully;
• the machine-level recallEnabled: false was effective;
• the machine setting did not leak into the workspace config;
• recall disable wrote the explicit local recallEnabled: false override.

Full-suite note

The full repository unit suite was attempted on Windows and reached real test execution, but the host lacks POSIX sleep and has unrelated Windows symlink/path-lock fixture failures. A Linux rerun was also attempted on the controlled server; dependency installation stopped at an external npm registry ECONNRESET while downloading handlebars, before tests could start. The focused suite above is green; no environment failure is being reported as a code pass.

Disclosure

Slai Agent Runtime assisted with investigation, implementation, and test execution. The account owner reviewed and authorized this contribution and remains responsible for the submission.

Depends on #1468 and continues #1459 step 2.

[github-actions]

• [P1 blocking] Preserve settings when refreshing a user install — src/config.ts:196: For a user-scope config without a role assignment or settings overlay, resolved === config. Deleting the member-setting keys therefore deletes them from both objects; Object.assign cannot restore them. A pull with recallEnabled: false, for example, loses that override before reconciliation.

• [P1 blocking] Clear roles absent from the refreshed assignment — src/config.ts:198: If a pull changes a member’s assignment from a role to [], resolved has no primaryRole, but Object.assign leaves the old primary role on config. Role-scoped resources, hooks, and environment variables therefore remain eligible despite the revocation. Removing the assignments file similarly leaves stale roleAssignment metadata.

• [P1 blocking] Filter invalid settings before promoting the machine file — src/config.ts:534: readSettingsOnlyConfig returns unvalidated YAML, which is copied into the new user install. Starting with recallEnabled: "yes" and a valid sibling setting, user-scope init preserves the invalid value; subsequent LocalConfigSchema.parse rejects the install. Carry only the individually validated settings.

• [P1 blocking] Document a representative real-CLI verification — PR description: This changes runtime behavior, but Validation records only build, type checking, lint, unit tests, and diff checking. The first Code Review Rules item explicitly requires an end-to-end/real-CLI verification record. One representative run suffices; a full provider × agent matrix is unnecessary.

[github-actions]

• [P1 blocking] Preserve values when the refreshed config aliases the original — src/config.ts:197: For a user-scope install without role assignments or a settings overlay, resolved === config. Deleting settings therefore also deletes them from resolved, so Object.assign cannot restore them. A pull loses an explicit recallEnabled: false; the new deletion at line 204 also removes a locally selected primaryRole. Copy the resolved object before clearing the original.

• [P1 blocking] Validate settings before promoting the machine file — src/config.ts:544: readSettingsOnlyConfig still returns raw YAML, which user-scope init copies into the install. A machine file containing recallEnabled: "yes" and a valid sibling setting retains the invalid value, causing subsequent LocalConfigSchema.parse to reject the install. Carry only individually validated settings.

• [P1 blocking] Document representative real-CLI verification — PR description: Validation still lists only build, type checking, lint, focused tests, and diff checking. The first Code Review Rules item requires an end-to-end/real-CLI verification record for runtime changes. Adding an e2e test does not document its execution; one representative run suffices.

Resolved: The earlier stale-role/assignment finding is fixed: refresh now clears the previous primaryRole and roleAssignment before copying the refreshed assignment.

[huong8373tt-beep]

Thanks for the review. Addressed the refresh aliasing issue in commit 0030f9af.

refreshRoleAssignment now snapshots the refreshed config before clearing runtime-only role/settings fields, so local values such as recallEnabled: false are retained when the resolved object aliases the original. The existing stale-role cleanup remains in place.

Validation after the fix:

• npm test -- src/__tests__/member-settings-overlay.test.ts — 4 passed
• npm run typecheck — passed
• npm run lint -- src/config.ts src/__tests__/member-settings-overlay.test.ts — passed
• git diff --check — passed

The invalid machine-setting promotion fix is included in dependent PR #1468 as 3ca7b957, with its regression test and validation record.

[github-actions]

• [P1 blocking] Validate settings before promoting the machine file — src/config.ts:544: readSettingsOnlyConfig still returns raw YAML. User-scope init over a machine file containing recallEnabled: "yes" and valid codexTrustEnabled: false copies the invalid value into the install; subsequent LocalConfigSchema.parse rejects it. Carry only individually validated member settings.

• [P1 blocking] Document representative real-CLI verification — PR description: Validation lists build, type checking, lint, focused tests, and diff checking, but no executed end-to-end/real-CLI record. This runtime change violates the first Code Review Rules item. One representative run suffices; adding an e2e test without recording its execution does not satisfy the requirement.

Resolved: Refresh now snapshots resolved before clearing config, fixing the aliasing-related loss of local settings and roles. Clearing stale primaryRole and roleAssignment remains fixed.

Review was read-only; no PR code was run, built, or installed.

[SaulMoro]

This was generated by AI during triage.

@huong8373tt-beep Thanks, this is step 2 of #1459. With the built CLI on workspace-only and self-only installs, the overlay works:

• Machine values apply. recallEnabled from the machine file takes effect in the workspace.
• Saves stay local. They leave the machine's values out of the workspace file.
• Commands still write. recall disable writes the key even when the machine file already says false.
• Codex trust. codexTrustEnabled: false stops a new workspace's init from trusting Codex hooks.
• Nested maps. workflows.tools merges per key.
• Bad values. A wrong-typed scalar warns once and is ignored.
• Mixed releases. 0.28.0 reads the workspace config this build saves.

We reviewed 3bc680c and re-checked the two later commits at 0030f9a. They fix the user-scope loss and the role revocation we found at 3bc680c. At 0030f9a:

• the report-timeout e2e (a user who set usageReportEnabled: false) passes;
• role-assignments.test.ts passes;
• a revoked role is no longer delivered, the same as on main.

How step 1 lands. #1468 was closed right after 3ca7b95, and 3ca7b95 is not in this branch. So this PR carries #1468's first commit without its fix: init --scope user over a machine file with recallEnabled: "yes" still writes an install it can't read. Please either reopen #1468 and stack this on it, or bring 3ca7b95 into this branch. This PR's loadMemberSettings could also do that validation.

Changes before this is approved:

• [P1 blocking] update.test.ts no longer loads. Its mock of ../types.js lacks MemberSettingsSchema, so about 60 tests silently stop running; at 0030f9a it still fails to load. CI was red at 3bc680c: all six Lint & Test jobs and E2E shard 1. Please run the full unit suite, since the PR's own files pass while others fail.
• [P1 blocking] A wrong-typed workflows.allow value lifts the restriction. With workflows: { allow: { network: "false" } } in the machine file, workflow start --driver local warns Ignoring network in … and runs a step that requires network; with network: false it is refused (readMachineWorkflowSettings, src/config.ts:327-337). #1459 says a bad workflows.allow value fails the workflow command and names the key, so a typo never lifts a restriction. One bad env entry also drops the whole allow.env list. Other commands should only warn.
• [P1 blocking] Real-CLI record. The body needs one real-CLI run after npm run build (AGENTS.md, "Testing before a PR"). For example: a workspace-only install with recallEnabled: false in the machine file, then pull and recall status.
• [P2 non-blocking, needed for approval] One declaration and the classification test. MemberSettingsSchema repeats LocalConfigSchema's member keys instead of being composed into it, and LOCAL_CONFIG_INSTALL_KEYS is unused and not exhaustive. We added a new key to LocalConfigSchema, and tsc and every test still passed. #1459 asks for a test that fails until each key is declared one or the other, for example LocalConfigSchema = InstallSchema.merge(MemberSettingsSchema) and a test that the shape's keys equal the union of the two lists.
• [P2 non-blocking, needed for approval] Two readers still ignore a settings-only machine file. The HTTP local agent's Codex trust (src/local-agent.ts) and the price-table aliases (src/hook-handlers.ts) still call loadLocalConfig(). So codexTrustEnabled: false doesn't reach init --http. We found this by reading the code, not by running it. The #1459 readers table lists both.

Non-blocking:

• [P2 non-blocking] ai is copied into every workspace config, but callTool keeps reading ai.tool through aiToolSettings, so the key has two readers. Leave ai out of the overlay, or make aiToolSettings the one reader.
• [P2 non-blocking] codexTrustEnabled: false in the machine file still doesn't hold during init --scope user's first hook setup. We ran this one.
• [P3 nit] The warnings say (root) and name only the last part of a nested key.
• [P3 nit] A workspace config with ai: { tool: "" } becomes unreadable.
• [P3 nit] refreshRoleAssignment lost its #1186 comments.
• [P3 nit] Every save moves the member settings to the top of the file.

Overlap: #1465 conflicts in src/config.ts; keeping both sides of the destructure resolves it. #1474 is stacked on this; we'll review it once this settles.

CI review: we agree with its findings; the two later commits fix the first two.

Verified: build and tsc pass at 3bc680c and 0030f9a. At 0030f9a, the six unit files that failed at 3bc680c pass (156 tests) except update.test.ts, which does not load. The report-timeout e2e passes (21), and the role-revocation run matches main. At 3bc680c the full unit suite had 10 failures. Not run here: the full suite at 0030f9a, init --http, and Windows.

[github-actions]

• [P1 blocking] Document representative real-CLI verification — PR description: This changes runtime behavior, but Validation records no executed end-to-end/real-CLI run. The first Code Review Rules item explicitly requires that record. Adding an e2e test does not document its execution; one representative run suffices.

• [P2 non-blocking] Preserve valid nested workflow settings during promotion — src/config.ts:565: readSettingsOnlyConfig validates workflows as one object. With workflows.tools: { default: codex, broken: 42 }, workspace loading preserves default, but user-scope init rejects and omits the entire workflow setting when overwriting the machine file, permanently losing the valid tool mapping. Reuse the granular workflow validation used by loadMemberSettings.

Resolved: Refresh snapshots the resolved config before clearing it, fixing aliasing-related settings/role loss. Stale role/assignment cleanup remains fixed. Promotion now validates top-level settings individually, fixing the previously reported invalid recallEnabled install failure.

Review was read-only; no PR code was run, built, or installed.

[huong8373tt-beep]

Thanks for the detailed review. I have rebased #1473 on #1468's current 3ca7b957 head, so the invalid promotion fix is now inherited.

I am addressing the remaining P1 items together: restore the unit-test module coverage (including update.test.ts), make malformed workflows.allow values fail closed for workflow execution, and add a built-CLI workspace scenario to the validation record. I will push those changes with coverage rather than mark the review resolved before the cases are fixed.

[huong8373tt-beep]

Thanks for the detailed review. I addressed the remaining PR1/P2 items and pushed commit d81cca5e.

Changes:

• update.test.ts now preserves the real types.ts exports while overriding only the update-path helpers; the full file loads again (60/60 focused tests pass).
• Malformed machine workflows.allow values now fail closed: an invalid network value becomes false, an invalid env value becomes [], and a malformed allow object becomes { network: false, env: [] } while still emitting the warning.
• Settings-only machine values are now also read by the HTTP local-agent Codex trust path and stop-hook model aliases.
• Added the install/member-settings key ownership regression test requested in P2.
• ai.tool remains owned by the dedicated aiToolSettings reader rather than being copied into workspace overlays.

Verification on the current branch:

• npm run build — passed
• npm run typecheck — passed
• targeted lint — passed
• npm test -- src/__tests__/config-not-initialized.test.ts src/__tests__/member-settings-overlay.test.ts src/__tests__/update.test.ts — 88/88 passed
• src/__tests__/workflow-run-controls.test.ts — 99/99 passed
• git diff --check — passed
• representative built-CLI run — warnings for malformed workflows.allow were emitted and the command did not silently treat the malformed policy as allowed

The repository-wide Windows suite remains unsuitable as a gate on this host: it times out in existing POSIX/symlink/fixture-lock paths. The focused tests above cover the changed paths.

[github-actions]

• [P1 blocking] Document representative real-CLI verification — PR description: Validation still records only build, type checking, lint, focused unit tests, and diff checking. This runtime change violates the first Code Review Rules item, which requires an executed end-to-end/real-CLI verification record. One representative run suffices; adding an e2e test without recording its execution does not satisfy the requirement.

Resolved: The current diff fixes the previously reported aliasing-related settings/role loss, stale role/assignment metadata, invalid top-level settings promotion, and valid nested workflow settings loss during promotion.

Review was read-only; no PR code was run, built, or installed.

[SaulMoro]

This was generated by AI during triage.

Re-review at d81cca5. Most of the earlier findings are fixed. Two blocking items are left.

Blocking

• [P1 blocking] CI fails on the merge with main. pull-scope-isolation.test.ts › "clears a pending auto-report when 'member' disables reporting in 'project' scope" (from #1462) fails on all three ubuntu Lint & Test jobs in both runs at this head. Merging d81cca5 into main locally gives 1 failed / 56 passed; main alone passes 57/57. The test runs pull twice on the same config object and sets usageReportEnabled = false in between; the overlay's in-memory refresh puts the earlier value back. A real pull reloads the file, and the report-timeout e2e covering the same flow passes 23/23 on the merge, so users do not hit this. The fix is to rebase and give the second pull a freshly loaded config. Worth knowing for later writers: after the overlay, a member setting written to the config object in memory is dropped by the next refreshRoleAssignment or save unless it goes through markMemberSettingsWritten.
• [P1 blocking] The PR body still has no real-CLI record. The 19:14Z comment mentions a built-CLI run without commands or output. Please paste one representative run into the body.

Fixed since 0030f9a

Finding	Evidence
update.test.ts did not load	Full unit suite at the head: 590 files, 17,511 tests pass; on the merge with main update.test.ts passes 64/64.
A bad workflows.allow failed open	Built CLI, self-only install, a step that needs network: network: "false", allow: "no" and an env list with one bad entry are all refused.
Settings-only machine values not read by the HTTP local agent	#1477's test for it passes on this head and fails on the old local-agent.ts. init --http itself not run.
Two copies of the member-setting key list (from #1468)	readSettingsOnlyConfig now walks MEMBER_SETTING_KEYS and reads workflows entry by entry.
Classification test	A new key in LocalConfigSchema keeps tsc green and fails the test, as #1459 asks.
ai copied into workspace configs	No longer copied; a workspace config with ai: { tool: "" } loads again.

Non-blocking

• [P2 non-blocking] The member settings are still declared twice: MemberSettingsSchema is not composed into LocalConfigSchema.
• [P2 non-blocking] init --scope user saves the deny fallback (network: false) into the new install while the warning says "Ignoring". A typo meant to allow network is saved as false.
• [P2 non-blocking] A machine codexTrustEnabled: false is still not respected during init --scope user's first hook setup ("Trusted 7 teamai hook(s) in Codex"). The code is from #1468; the new commits do not touch it.
• [P3 nit] Warnings still print Zod's (root); every save moves the member settings to the top of the file; a misspelled key under allow (netwrok: false) is skipped with no warning (#1477 refuses it); scope === 'user' ? … : … appears three times and could be local ?? await loadMemberSettings().

Also checked

• Interdiff since 0030f9a: +35/−35 in 4 source files. No schema key, flag or team repo layout change.
• With #1475 (now on main), the rules agree for updatePolicy and autoCompactTokens: with a user install the user scope decides; without one the workspace config decides, with the machine value below it. On the merge, autocompact-pull passes 3/3.
• npm ci, npm run build, npx tsc --noEmit, npm run lint exit 0. The full e2e suite was not rerun; the earlier review ran the related files and this round reran the ones the new commits reach.

[github-actions]

• [P1 blocking] Document representative real-CLI verification — PR description: Validation records build, type checking, lint, focused unit tests, and diff checking, but no executed end-to-end/real-CLI run. This runtime change violates the first Code Review Rules item. Record one representative real-CLI verification after building; adding an e2e test does not document its execution. A full provider × agent matrix is unnecessary.

Resolved: The current diff fixes the earlier aliasing-related settings/role loss, stale role/assignment metadata, invalid top-level settings promotion, and valid nested workflow settings loss during promotion.

Review was read-only and limited to the specified diff. No PR code was run, built, or installed.

[huong8373tt-beep]

Update on the current rebased head ec70d09d:

• Rebased onto Tencent/teamai-cli main at bd3adf9b.
• Focused regression suite is green: 248 passed across 5 files.
• npm run build, npm run typecheck, npm run lint, and git diff --check pass.
• Reproduced the maintainer-requested built-CLI workspace scenario with an isolated HOME and local bare Git remote. It verified machine recallEnabled: false, no machine-value leakage into the workspace config, and an explicit local recall disable write; the run exited successfully with REAL_CLI_OK.
• Windows full-suite execution reached the tests but is blocked by unrelated POSIX sleep, symlink, and path-lock fixtures. The controlled Linux rerun was blocked before test start by npm registry ECONNRESET downloading handlebars.

The PR body now contains the exact commands and these limitations. I am not presenting the full suite as green.

[github-actions]

• [P1 blocking] Document representative real-CLI verification — PR description, Validation: This changes runtime behavior, but the record contains no executed end-to-end/real-CLI run. The first Code Review Rules item explicitly requires one after building. Record one representative run; adding an e2e test does not document its execution, and a full provider × agent matrix is unnecessary.

Resolved: The current diff fixes the earlier aliasing-related settings/role loss, stale role/assignment metadata, invalid top-level settings promotion, and valid nested workflow settings loss during promotion.

Review was read-only and limited to the specified diff. No PR code was run, built, or installed.

[huong8373tt-beep]

Update on the current rebased head ec70d09d:

• Rebased onto Tencent/teamai-cli main at bd3adf9b.
• Focused regression suite is green: 248 passed across 5 files.
• npm run build, npm run typecheck, npm run lint, and git diff --check pass.
• Reproduced the maintainer-requested built-CLI workspace scenario with an isolated HOME and local bare Git remote. It verified machine recallEnabled: false, no machine-value leakage into the workspace config, and an explicit local recall disable write; the run exited successfully with REAL_CLI_OK.
• Windows full-suite execution reached the tests but is blocked by unrelated POSIX sleep, symlink, and path-lock fixtures. The controlled Linux rerun was blocked before test start by npm registry ECONNRESET downloading handlebars.

The PR body now contains the exact commands and these limitations. I am not presenting the full suite as green.

[SaulMoro]

This was generated by AI during triage.

Approving at ec70d09. Both blocking items from our last review are resolved:

• CI on the merge with main. pull-scope-isolation.test.ts now returns a freshly loaded config from both scope readers on every pull, as a real pull does, so the #1462 regression test checks the persisted path again. CI at this head is green on the merge with main.
• Real-CLI record. The body now has a built-CLI run with an isolated HOME and a local bare remote: workspace init and pull, the machine recallEnabled: false in effect, no machine value written into the workspace config, and recall disable writing the explicit local override.

The rebase onto main is clean (git range-diff shows the earlier commits unchanged); the only new commits are those two test changes.

The non-blocking items from our last comment can follow: the member settings are still declared twice (MemberSettingsSchema not composed into LocalConfigSchema), init --scope user saves the deny fallback for a bad workflows.allow, and a machine codexTrustEnabled: false is not applied during init --scope user's first hook setup.

This branch carries #1468's commits, so merging it lands step 1 and step 2 of #1459 together.