Repository navigation
Honor active harness config homes in setup-pstack [#120] - #123
Conversation
Keep real HOME and USER while preparing redirected, credential-only profiles and checking setup artifacts without claiming a live run. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Resolve nonempty config-home overrides at the existing harness mapping boundary and reuse that path throughout setup and integration rendering. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Guard the canonical mapping and default/empty/spaced resolution without daily writes, and record the intentional port divergence for the patch ledger. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Merge Protections🟢 All 2 merge protections satisfied — ready to merge. Show 2 satisfied protections🟢 Queue ready pull requestsAutomatically queue non-draft pull requests targeting main after verify, Unfret, and live-gate pass on the head.
🟢 🚦 Auto-queueWhen all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Resolve the current parent's config home once: Claude Code uses nonempty `CLAUDE_CONFIG_DIR`, otherwise `$HOME/.claude`; Codex uses nonempty `CODEX_HOME`, otherwise `$HOME/.codex`. The equivalent quoted shell expressions are: | ||
|
|
||
| ```bash | ||
| "${CLAUDE_CONFIG_DIR:-$HOME/.claude}" | ||
| "${CODEX_HOME:-$HOME/.codex}" |
There was a problem hiding this comment.
If CLAUDE_CONFIG_DIR or CODEX_HOME is relative and the operator changes directories after starting the parent, setup can write to a different home from the one the parent uses. Its readback can still succeed, but the parent will not load the model sheet. Make the chosen home absolute before setup reuses it.
| printf '%s\n' '# Fixture instructions' 'Preserve this unrelated text exactly.' > "$run/sentinel" | ||
| cp "$run/sentinel" "$run/claude config/CLAUDE.md" |
There was a problem hiding this comment.
The fixture starts with a new CLAUDE.md, so it never checks an upgrade from the old @~/.claude/pstack-models.md line to the new resolved-path line. Add an upgrade case that starts with the old line and checks that only one working include remains. Without it, the evidence run can miss a duplicate or stale import.
| # Config-home port invariant (#120): guard upstream merges against daily-home writes. | ||
| config_mapping="$repo/plugins/pstack/skills/poteto-mode/references/codex-tools.md" | ||
| config_home_bad="" | ||
| if grep -nE '~/(\.claude|\.codex)|\$HOME/(\.claude|\.codex)' "$setup"; then |
There was a problem hiding this comment.
The default-home guard matches ~/ and $HOME/, but not ${HOME}/.claude or ${HOME}/.codex. A future setup edit could use the braced spelling to write to a daily home while this check stays green. Match that spelling too.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
✅ Accepted riskAccepted by ericlitman: GH-120 (authority 48fa77b) 🟡 Medium · Imported-sheet verifier rejects required migrations 🟡 Medium · Verifier accepts extra integration content after sentinel 🟡 Medium · Alias-led panel rows bypass assignment verification Advisories (6)
DetailsRun: @unfret review · @unfret status · @unfret help |
![unfret-eal[bot]](https://avatars.githubusercontent.com/u/28211?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛑 1 blocking finding
🟡 Medium · Claude include rerun lacks a rule for existing sheet imports
Check
| ### 8. Wire it in | ||
|
|
||
| Render the parent integration in memory before either write. On Claude, the integration is the single `@~/.claude/pstack-models.md` include in `~/.claude/CLAUDE.md`. On Codex, it is the exact sheet bytes between one `<!-- pstack:models:begin -->` and `<!-- pstack:models:end -->` pair in `~/.codex/AGENTS.md`. Replace that whole bounded block on a rerun. Insert one block at the end on first run. If either marker is missing, duplicated, or reversed, stop and report inconsistent state instead of guessing a boundary. | ||
| Render the parent integration in memory before either write. On Claude, the integration is the single `@` include of the actual resolved `<config-home>/pstack-models.md` path (backslash-escape spaces, never leave a variable or placeholder) in `<config-home>/CLAUDE.md`. On Codex, it is the exact sheet bytes between one `<!-- pstack:models:begin -->` and `<!-- pstack:models:end -->` pair in `<config-home>/AGENTS.md`. Replace that whole bounded block on a rerun. Insert one block at the end on first run. If either marker is missing, duplicated, or reversed, stop and report inconsistent state instead of guessing a boundary. |
There was a problem hiding this comment.
🟡 Medium · Claude include rerun lacks a rule for existing sheet imports
The diff deleted the fixed Claude integration text "the single @~/.claude/pstack-models.md include in ~/.claude/CLAUDE.md". Step 8 now renders @<actual resolved path>. The Codex path has a find/replace/stop rule, but this Claude path gives no rule to find, replace, or reject an existing pstack-models.md import. Scenario 1: an upgrading default-home user reruns setup. CLAUDE.md still holds @~/.claude/pstack-models.md, while the render is @/Users/u/.claude/pstack-models.md. The lines no longer match, so the agent can append a second import. The rerun is then not byte-identical, which contradicts CHANGES' "default-home behavior unchanged". Scenario 2: a CLAUDE_CONFIG_DIR profile created by copying ~/.claude keeps an import of the old home's sheet. Setup appends the new home's import, so both sheets load with conflicting role descriptors. Dispatch can then pick the wrong route. The new fixture starts from a sentinel with no prior import, so it never exercises this path.
Keep default-home upgrades byte-identical and replace copied-profile imports instead of loading two model sheets. Require deterministic zero/one/many import handling in the config-home invariant. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🛑 1 blocking finding
🟡 Medium · Redirected Claude imports break on paths containing `#`
Check
| Render the parent integration in memory before either write. On Claude, the integration is the single `@~/.claude/pstack-models.md` include in `~/.claude/CLAUDE.md`. On Codex, it is the exact sheet bytes between one `<!-- pstack:models:begin -->` and `<!-- pstack:models:end -->` pair in `~/.codex/AGENTS.md`. Replace that whole bounded block on a rerun. Insert one block at the end on first run. If either marker is missing, duplicated, or reversed, stop and report inconsistent state instead of guessing a boundary. | ||
| Render the parent integration in memory before either write. | ||
|
|
||
| On Claude, the integration is the one `@` import line in `<config-home>/CLAUDE.md` whose target's basename is `pstack-models.md`, regardless of its existing directory or path spelling. When `<config-home>` is the default home, render exactly `@~/.claude/pstack-models.md`, preserving the legacy text. Only when `CLAUDE_CONFIG_DIR` redirects the home, render `@` followed by the absolute resolved `<config-home>/pstack-models.md` path with each space backslash-escaped, never a variable or placeholder. On a rerun, replace that one line in place, preserving all unrelated bytes. If zero matching import lines exist, append one. If more than one exists, stop and report inconsistent state before either write; do not append another import or guess which one to replace. |
There was a problem hiding this comment.
🟡 Medium · Redirected Claude imports break on paths containing #
For CLAUDE_CONFIG_DIR=/tmp/claude#work, a rerun replaces a working @./pstack-models.md import with @/tmp/claude#work/pstack-models.md. Claude Code's import parser treats # as a fragment separator and resolves this to /tmp/claude, not the sheet. Fresh sessions therefore stop loading the configured role choices even though setup's write/readback checks pass. Confirmed against the parsers in Claude Code 2.1.197 and 2.1.289. The same rendering rule also appears at setup line 18 and codex-tools.md line 22.
Use a sibling-relative model-sheet import so directory characters cannot prevent loading, while preserving legacy default-home imports and deterministic reruns. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Live test, Codex half (head 239a73c)Result: the Codex half passes. The Claude half has not run yet, so I have not set the
Configuration note: the default Codex sandbox blocked the external Opus runner's network access (ENOTFOUND). So I set Still pending: the same live test from Claude Code with a redirected |
Keep the config-home correction alongside the host sandbox prerequisites added by #125 without changing either implementation. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Live test, Codex half (head 536319d)Result: the Codex half passes on this head, after the
Still pending: the Claude Code half, which needs a parent Claude session authenticated inside a redirected |
There was a problem hiding this comment.
🛑 2 blocking findings
🟡 Medium · Rerun re-points a cross-directory sheet import without loading that sheet
🟡 Medium · Verifier accepts extra conflicting integration content
Check
| Render the parent integration in memory before either write. On Claude, the integration is the single `@~/.claude/pstack-models.md` include in `~/.claude/CLAUDE.md`. On Codex, it is the exact sheet bytes between one `<!-- pstack:models:begin -->` and `<!-- pstack:models:end -->` pair in `~/.codex/AGENTS.md`. Replace that whole bounded block on a rerun. Insert one block at the end on first run. If either marker is missing, duplicated, or reversed, stop and report inconsistent state instead of guessing a boundary. | ||
| Render the parent integration in memory before either write. | ||
|
|
||
| On Claude, the integration is the one `@` import line in `<config-home>/CLAUDE.md` whose target's basename is `pstack-models.md`, regardless of its existing directory or path spelling. When `<config-home>` is the default home, render exactly `@~/.claude/pstack-models.md`, preserving the legacy text. Only when `CLAUDE_CONFIG_DIR` redirects the home, render exactly `@./pstack-models.md`. This relative import resolves from the importing file's directory, where the sheet also lives, so the import line contains no config-directory characters. On a rerun, replace that one line in place, preserving all unrelated bytes. If zero matching import lines exist, append one. If more than one exists, stop and report inconsistent state before either write; do not append another import or guess which one to replace. |
There was a problem hiding this comment.
🟡 Medium · Rerun re-points a cross-directory sheet import without loading that sheet
Step 8 now treats an @ import with basename pstack-models.md as the integration "regardless of its existing directory" and replaces it in place. Step 2, per line 10, loads current state only from <config-home>/pstack-models.md, so the sheet the existing import actually loads is never read. Scenario: CLAUDE_CONFIG_DIR=/work/claude, and /work/claude/CLAUDE.md holds @~/.claude/pstack-models.md. This happens with a copied profile, or with a manual workaround for #120 that base setup kept updating. The customized sheet lives at ~/.claude/pstack-models.md. /work/claude/pstack-models.md is either absent or a stale copy. Setup then presents first-run defaults, or the stale copy, as the current assignments, with keep as the default. After confirmation it rewrites the import to @./pstack-models.md. Fresh sessions stop loading the customized roles and efforts; for example, a bug-fix lane customized to claude:opus reverts to Sol. This breaks step 2's rule against resetting a customized sheet. On base, setup read and rewrote ~/.claude/pstack-models.md, so that import kept loading the user's configuration. The same rule is repeated at codex-tools.md lines 16–22.
| if (!text.startsWith(sentinel)) fail('unrelated integration bytes changed'); | ||
| if (meta.harness === 'claude') { | ||
| const includes = text.split('\n').filter(line => line.startsWith('@') && line.includes('pstack-models.md')); | ||
| if (includes.length !== 1 || includes[0] !== '@./pstack-models.md') fail('Claude include is not the single relative sheet import'); | ||
| } else { | ||
| const begin = '<!-- pstack:models:begin -->'; | ||
| const end = '<!-- pstack:models:end -->'; | ||
| if (text.split(begin).length !== 2 || text.split(end).length !== 2 || text.indexOf(end) <= text.indexOf(begin)) fail('Codex block markers missing, duplicated, or reversed'); | ||
| const block = text.slice(text.indexOf(begin) + begin.length + 1, text.indexOf(end)); | ||
| if (!Buffer.from(block).equals(bytes)) fail('Codex block differs from model sheet bytes'); |
There was a problem hiding this comment.
🟡 Medium · Verifier accepts extra integration content after sentinel
Carried v1:7d4d3bf1 is still present. The verifier checks only the sentinel prefix and then either one @-led pstack import (Claude) or one marker block (Codex). It still prints ok for: (1) a Claude file with the sentinel, @./pstack-models.md, and bug-fix: claude:opus@max; (2) one that also keeps an indented @~/.claude/pstack-models.md, which Claude still imports but startsWith('@') ignores; (3) a Codex AGENTS.md with a stray row outside the block. It therefore certifies integrations that contradict or double-load the sheet, against step 8's single import/block plus preserved bytes. Also reported for this defect: - Verifier accepts conflicting integration suffixes (tests/setup-config-home-repro.sh:115-124): The verifier only requires the integration to start with the sentinel and contain the expected import or bounded block. Because preparation seeded no other suffix, a generated file containing the valid integration plus a stale row such as bug-fix: claude:opus@max still prints ok, even though that global instruction can conflict with the sheet. - Verifier accepts conflicting content outside the integration (tests/setup-config-home-repro.sh:115-124): Verification checks only the sentinel prefix and the presence of one expected import or bounded block; it never constrains the remaining bytes. A Claude file containing the valid relative import followed by a stale bug-fix: claude:opus@max instruction, or a Codex file with that row outside the valid block, passes even though the fixture initially contained only the sentinel and the extra global routing can contradict the generated sheet. - Verifier accepts conflicting integration suffixes (tests/setup-config-home-repro.sh:115, tests/setup-config-home-repro.sh:59-68, tests/setup-config-home-repro.sh:114-123, tests/setup-config-home-repro.sh:115-118, tests/setup-config-home-repro.sh:115-124, tests/setup-config-home-repro.sh:117-124): Checking only that the integration starts with the sentinel leaves all later non-integration content unconstrained. In both harnesses, the required import or bounded block followed by an unbounded row such as bug-fix: claude:opus@max passes verification, even though that global instruction can conflict with the model sheet. Also reported for this defect: - Verifier accepts conflicting content outside the integration (tests/setup-config-home-repro.sh:115-124): Only the sentinel prefix and the required import or bounded block are checked. A Claude integration containing the valid relative import plus a stale model-routing row, or a Codex integration containing the exact block plus such a row outside it, passes verification even though the extra global instruction can contradict the generated sheet. - Verifier accepts extra or stale content outside the integration (tests/setup-config-home-repro.sh:115-124): Still present at this head (carried v1:7d4d3bf1). Lines 115-124 check only three things: the sentinel prefix, one line that starts with @ and equals @./pstack-models.md (Claude), and one matching marker pair (Codex). Everything else after the sentinel is unconstrained. Verify exits 0 with ok for a redirected CLAUDE.md of sentinel + @./pstack-models.md + bug-fix: claude:opus@max. It also passes one that keeps an indented @~/.claude/pstack-models.md, which Claude still imports but startsWith('@') does not count. A Codex AGENTS.md with a stray row outside the block passes the same way. The helper therefore certifies integrations that contradict or double-load the sheet, against step 8's rule of a single import or block plus preserved bytes. - Verifier accepts extra conflicting integration content (tests/setup-config-home-repro.sh:59-68, tests/setup-config-home-repro.sh:114-123, tests/setup-config-home-repro.sh:115-118, tests/setup-config-home-repro.sh:117-124): The verifier only requires the sentinel as a prefix and then checks for one valid import or bounded block. An integration containing the correct import/block plus an unbounded stale model row still prints ok, even though that extra global instruction can conflict with the model sheet and violates step 8's single integration shape. Also reported for this defect: - Verifier accepts stale model instructions outside the integration (tests/setup-config-home-repro.sh:114-123): For either harness, an integration containing the untouched sentinel, the required relative import or exact bounded block, and an extra unbounded row such as bug-fix: claude:opus@max still passes. The prefix and branch checks never reject that suffix, so a sheet routing bug-fix to Sol can conflict with the global instruction while --verify exits 0 and prints ok. - Verifier still accepts extra conflicting integration content (tests/setup-config-home-repro.sh:115-118): Carried demand v1:7d4d3bf1 is still present at this head. Lines 115-118 only check that the integration starts with the sentinel and contains one @./pstack-models.md import (Claude) or one marker pair matching the sheet (Codex). Nothing else after the sentinel is constrained. Scenario: the redirected CLAUDE.md is the sentinel, then @./pstack-models.md, then a stale bug-fix: claude:opus@max line. Verify exits 0 and prints ok. The same holds for a Codex AGENTS.md with a stray row outside the block. Step 8 allows only the single import line or bounded block beyond the preserved bytes, so the acceptance helper certifies an integration that can contradict the sheet. - Verifier accepts conflicting integration rows (tests/setup-config-home-repro.sh:117-124): A Claude file with the required @./pstack-models.md import, or a Codex file with a byte-identical bounded block, still passes when an additional model row such as bug-fix: claude:opus@max appears outside that integration. Those stale global instructions can conflict with the sheet, yet --verify exits 0 and prints ok.
Now at tests/setup-config-home-repro.sh:115-124
Also affects:
Live tests on head d516ad3: both harnesses pass (not gating)I am not setting Claude Code
Codex
|
Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🛑 3 blocking findings
🟡 Medium · Valid imported-sheet migrations fail verification
🟡 Medium · Verifier accepts extra conflicting integration content
🟡 Medium · Assignment check skips alias-led multi-lane rows
Check
| if (fs.existsSync(imported)) { | ||
| const assignments = text => text.split('\n').filter(line => /^[^:#]+:\s*(?:claude:|codex:|grok:|inherit-parent\s*$|auto\s*$)/.test(line)).map(line => line.trim()).sort(); | ||
| const before = assignments(fs.readFileSync(imported, 'utf8')); | ||
| if (!before.length || JSON.stringify(before) !== JSON.stringify(assignments(bytes.toString('utf8')))) fail('imported sheet assignments were not preserved'); |
There was a problem hiding this comment.
🟡 Medium · Imported-sheet verifier rejects required migrations
The exact assignment-list comparison rejects valid setup output. For example, an imported claude:claude-opus-4-6@max descriptor must normalize to claude:opus@max, and setup must also materialize missing roles and drop retired roles; each required transformation makes --verify fail with “assignments were not preserved,” blocking valid imported-sheet acceptance. Also reported for this defect: - Imported-sheet verify rejects setup's required migrations (tests/setup-config-home-repro.sh:111): Still present at this head (carried v1:3cd50c5e). Line 111 requires the redirected sheet's sorted assignment lines to equal the daily sheet's exactly. Setup must change some rows on load: it normalizes claude:claude-opus-* and claude:claude-fable-* to the rolling aliases, drops retired rows such as how critics, materializes missing role rows, and replaces out-of-matrix rows such as 1.4.x grok:grok-4.6. Step 7 (SKILL.md:91) explicitly permits the documented normalization. With such a daily sheet, a correct setup run fails --verify with 'imported sheet assignments were not preserved'. The operator cannot complete imported-sheet acceptance without editing daily config. The static 2-row fixture passes only because it is an output setup could never emit unchanged; setup would materialize the other roles. - Imported-sheet verifier rejects required migrations (tests/setup-config-home-repro.sh:111): The verifier requires the pre- and post-setup assignment lists to be exactly equal. A supported imported sheet containing versioned Fable/Opus descriptors, a retired role, or a missing documented role must be normalized, pruned, or completed by setup, so a correct run exits with imported sheet assignments were not preserved instead of passing acceptance. - Valid imported-sheet migrations fail verification (tests/setup-config-home-repro.sh:111, tests/setup-config-home-repro.sh:110): This exact assignment-list equality rejects transformations that setup explicitly requires. If the imported sheet uses a versioned Fable/Opus predecessor, omits a documented role, or contains a retired role, setup correctly normalizes, materializes, or drops rows, but --verify reports that assignments were not preserved. The static fixture avoids this by manually writing the same incomplete two-row sheet rather than setup's valid complete render. Also reported for this defect: - Imported-sheet verifier rejects required migrations (tests/setup-config-home-repro.sh:110): This comparison requires the old and new assignment-line sets to be identical. For an imported sheet with versioned Fable/Opus predecessors, a retired role, or missing current roles, setup is required to normalize, drop, or materialize rows; the correct output therefore fails with imported sheet assignments were not preserved. The static fixture's two-row input likewise could not be emitted unchanged by a real setup, which must materialize the remaining roles. - Imported-sheet verify rejects setup's mandated normalizations (tests/setup-config-home-repro.sh:111): Line 111 requires the redirected sheet's assignment lines to exactly equal the daily snapshot's. Setup step 2 is required to change some rows: it migrates versioned claude:claude-opus-* descriptors, drops retired rows such as how critics, materializes missing role rows, and requires replacing out-of-matrix rows. CHANGES 1.5.0 notes that a 1.4.x grok:grok-4.6 row persists until setup asks, and also tells users to delete role lines. With such a daily sheet, a correct setup run fails with 'imported sheet assignments were not preserved'. The operator cannot complete imported-sheet acceptance without editing daily config. This fails closed. The static case passes only because it treats a 2-row sheet as correct output, which setup never produces. Also reported for this defect: - Imported-sheet verify rejects setup's required migrations (tests/setup-config-home-repro.sh:111): Carried v1:3cd50c5e is still present. Line 111 requires the sorted assignment lines of the daily sheet and the redirected sheet to be exactly equal. Setup is required to change rows on load: it normalizes claude:claude-opus-*, drops retired rows such as how critics, materializes missing roles, and forces replacement of out-of-matrix rows such as 1.4.x grok:grok-4.6. If the daily ~/.claude/pstack-models.md needs any of these, a correct setup run fails --verify with 'imported sheet assignments were not preserved'. Imported-sheet acceptance then cannot complete without editing daily config. The 2-row static fixture passes only because setup could never emit it unchanged. - Verifier rejects required imported-sheet migrations (tests/setup-config-home-repro.sh:111): For an imported sheet containing a versioned Fable/Opus descriptor, missing documented role, or retired role, setup is required to normalize, complete, or prune the map. This exact before/after assignment-list comparison therefore rejects correct output with imported sheet assignments were not preserved, blocking valid imported-sheet acceptance. - Exact comparison rejects valid imported-sheet migrations (tests/setup-config-home-repro.sh:111): The verifier requires the imported and generated assignment lines to be byte-equivalent after sorting. For an imported sheet containing a supported predecessor such as claude:claude-opus-4-6@max, a retired role, or a missing documented role, setup must normalize, drop, or materialize rows. That correct output therefore makes --verify fail with “assignments were not preserved,” blocking imported-sheet acceptance.
Now at tests/setup-config-home-repro.sh:111
Also affects:
| const bytes = fs.readFileSync(sheet); | ||
| const imported = path.join(run, 'imported-sheet'); | ||
| if (fs.existsSync(imported)) { | ||
| const assignments = text => text.split('\n').filter(line => /^[^:#]+:\s*(?:claude:|codex:|grok:|inherit-parent\s*$|auto\s*$)/.test(line)).map(line => line.trim()).sort(); |
There was a problem hiding this comment.
🟡 Medium · Alias-led panel rows bypass assignment verification
The extraction regex recognizes inherit-parent and auto only when they end the line. A supported panel such as arena runners: inherit-parent, codex:gpt-5.6-sol@max, grok:grok-4.7@xhigh is omitted from both snapshots. If setup drops the Codex lane while another ordinary row keeps the list nonempty, --verify still succeeds and falsely certifies changed fan-out. Also reported for this defect: - Alias-led multi-lane rows escape assignment comparison (tests/setup-config-home-repro.sh:109): Carried v1:dfc04d2c is still present. The line-109 regex accepts inherit-parent or auto only at end of line, so a permitted row like arena runners: inherit-parent, codex:gpt-5.6-sol@max, grok:grok-4.7@xhigh is excluded from both lists. Scenario: setup writes arena runners: inherit-parent, grok:grok-4.7@xhigh, dropping the Sol lane. Other rows keep before nonempty and the lists still compare equal, so verify prints ok despite the changed fan-out. - Alias-led panel rows bypass preservation checks (tests/setup-config-home-repro.sh:109): The regex recognizes inherit-parent and auto only when they end the line. A supported panel such as arena runners: inherit-parent, codex:..., grok:... is omitted from both snapshots; if setup drops the Codex lane while another recognized row keeps the list nonempty, verification still exits 0 and falsely certifies changed fan-out. - Alias-led panel rows bypass assignment verification (tests/setup-config-home-repro.sh:109): The extraction regex only accepts inherit-parent or auto when they end the line. A row such as arena runners: inherit-parent, codex:..., grok:... is therefore omitted before and after setup; if setup drops or changes one of its provider lanes, another recognized row satisfies the nonempty check and --verify still prints ok, falsely certifying changed fan-out. Also reported for this defect: - Assignment filter drops alias-led multi-lane rows (tests/setup-config-home-repro.sh:109): Still present at this head (carried v1:dfc04d2c). The line-109 regex keeps a row only when its value starts with claude:, codex: or grok:, or is exactly inherit-parent or auto. A permitted panel row such as arena runners: inherit-parent, codex:gpt-5.6-sol@max, grok:grok-4.7@xhigh is therefore excluded from both the before and after lists. If setup writes arena runners: inherit-parent, grok:grok-4.7@xhigh, the Sol lane is lost and fan-out drops from 3 to 2. Line 111 still compares equal lists and verify prints ok, certifying changed routing as preserved. - Alias-led multi-lane rows are skipped (tests/setup-config-home-repro.sh:109): The regex recognizes inherit-parent and auto only at end of line, so a panel such as arena runners: inherit-parent, codex:..., grok:... is excluded from both assignment lists. Removing the Codex lane therefore still lets --verify print ok, falsely certifying preservation despite changed fan-out and routing. - Assignment check skips alias-led multi-lane rows (tests/setup-config-home-repro.sh:109): The filter regex on line 109 anchors inherit-parent\s*$|auto\s*$. A panel row whose first lane is an alias is therefore dropped from both the before and after lists. Step 7 allows such rows, for example arena runners: inherit-parent, codex:gpt-5.6-sol@max, grok:grok-4.7@xhigh. Scenario: the imported daily sheet has that row and setup writes arena runners: inherit-parent, grok:grok-4.7@xhigh, dropping the Sol lane. The two lists still compare equal, so verify prints ok and certifies 'preserved' assignments that changed the panel's fan-out. The rerun snapshot only compares later runs, so nothing else catches it.
Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🛑 4 blocking findings
🔴 Critical · Dangling Claude import resets an existing customized sheet
🟡 Medium · Imported-sheet verifier rejects required migrations
🟡 Medium · Verifier accepts conflicting integration suffixes
🟡 Medium · Alias-led panel rows bypass assignment verification
Check
| ### 2. Load current state | ||
|
|
||
| Read the current parent-specific sheet when it exists. Before matrix validation, normalize only the rolling-alias predecessors that earlier pstack releases generated. A provider-qualified Claude model is migratable when its model component starts with `claude-fable-` or `claude-opus-` and the remaining revision contains only digits and hyphens. Replace that component in memory with `fable` or `opus`, preserving the provider, effort, role, and lane order. Record each original and normalized descriptor for the confirmation in step 7. This migration is valid loaded state and does not require a separate operator choice. | ||
| On Claude, if the single existing model-sheet import in `<config-home>/CLAUDE.md` points elsewhere, resolve its target relative to the importing file's directory (expanding `~` to the user's home), read current state from that imported sheet, and name the source file; if it is missing, report that and use the normal first-run path. Otherwise, read the current parent-specific sheet when it exists. Before matrix validation, normalize only the rolling-alias predecessors that earlier pstack releases generated. A provider-qualified Claude model is migratable when its model component starts with `claude-fable-` or `claude-opus-` and the remaining revision contains only digits and hyphens. Replace that component in memory with `fable` or `opus`, preserving the provider, effort, role, and lane order. Record each original and normalized descriptor for the confirmation in step 7. This migration is valid loaded state and does not require a separate operator choice. |
There was a problem hiding this comment.
🔴 Critical · Dangling Claude import resets an existing customized sheet
New step 2 (SKILL.md:36) covers the case where the single import points elsewhere and its target is missing. It says to 'use the normal first-run path' and never reads the existing <config-home>/pstack-models.md; codex-tools.md:16 repeats this. Scenario: a user moves ~/.claude to ~/.config/claude and sets CLAUDE_CONFIG_DIR. CLAUDE.md still holds the now-dangling @~/.claude/pstack-models.md, while ~/.config/claude/pstack-models.md holds customized rows (e.g. bug-fix: claude:opus@max). Rerunning setup to repair loading presents first-run defaults as current state, with keep as the default. After confirmation, step 8 overwrites the customized destination with defaults; snapshots are restored only on failure. The default home is also affected when a dangling @~/dotfiles/pstack-models.md import exists: base read and preserved ~/.claude/pstack-models.md, but now it is overwritten. This violates step 2's rule never to reset a customized sheet.
Step-back result: PR #123 after reviewed head 7 (abb325b)The step-back used the same three messages as before. The advisor was GPT-6 Astra, read-only, in a checkout of Why the earlier repairs didn't converge. Each one answered a single case of the same question: which existing sheet is "current" when the config home and the existing integration disagree. Head 5 added "follow an import that points elsewhere". Head 7 then found that a dangling import fell through to first-run defaults and overwrote a surviving local sheet. The root defect is the step-2 shortcut that treats a missing import target as a first run. The rule for setup step 2 (agreed)
Changes
Live acceptance for the next headInstalled behavior on the exact candidate, in both harnesses:
No product decision is outstanding. |
Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🛑 3 blocking findings
🟡 Medium · Imported-sheet verifier rejects required migrations
🟡 Medium · Verifier accepts extra integration content after sentinel
🟡 Medium · Alias-led panel rows bypass assignment verification
Check
|
Unfret waiver for head 6ef25f5. Ticket
The verdict-changing findings from heads 5 and 7, about which sheet step 2 reads, were resolved by the step-back rule now at this head. |
Live tests and live-gate (head 6ef25f5)This head carries the step-back rule for setup step 2 and is level with Installed version.
Surface:
|
Merge Queue Status
This pull request spent 9 seconds in the queue, including 1 second running CI. Required conditions to merge
|
Bring in ericlitman/open-pstack main at e46bc89: setup reads and writes its sheet in the active config home (ericlitman#120), with the config-home invariant and repro tests. The local Cursor and Kimi setup paragraph is kept. The runner is unchanged. Version 1.5.1-local.9. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of the ericlitman#123 merge found the runner reading ./config.toml for an empty CODEX_HOME, which skipped the Codex MCP switch-off and trust cleanup, and passing an empty KIMI_CODE_HOME to Seatbelt, which refuses it. Both now fall back to the default home, matching the config-home rule ericlitman#123 documents. The local.9 changelog entry now lists the default-home setup changes ericlitman#123 makes instead of calling them none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Closes #120.
Make setup-pstack place its model sheet and parent integration in the config home the active harness reads. The existing harness-path mapping now specifies nonempty
CLAUDE_CONFIG_DIR/CODEX_HOME, with$HOME/.claude/$HOME/.codexfallbacks for unset or empty values. Setup uses the resolved home consistently for reads, writes, snapshots, restoration, and readback, and renders Claude imports with spaces escaped.Add a safe redirected-home evidence fixture, a named static regression invariant, and provenance entries in CHANGES.md and UPSTREAM.md so upstream syncs preserve this port divergence. No changes under
.github/workflows/.Verification on f95670e
bun install --frozen-lockfile— passed.bun run testinplugins/pstack/skills/poteto-mode/scripts— 159 pass, 0 fail, 653 assertions across 11 files.bun run typecheckin the same directory — all three strict TypeScript projects passed with--noEmit.PSTACK_STATIC_ONLY=1 bash tests/skill-collision-repro.sh— all 14 static invariant checks passed, including unset/empty defaults and space-containing overrides without writes.claude plugin validate ./plugins/pstack— validation passed.bash -n tests/setup-config-home-repro.sh tests/skill-collision-repro.sh— passed.git diff --check— passed; working tree clean.These are local/static and fixture checks, not real-harness setup acceptance evidence. Local verification output is retained in
/tmp/pstack-120-*.login the build sandbox, not committed or uploaded.Operator live acceptance on the final head
Per the approved revision-5 build conditions, this PR is ready for review before live proof. The operator must run redirected Claude and Codex setup against the exact final head, retain native transcripts/provider receipts, verify artifacts, preservation of unrelated integration text and daily files, byte-identical reruns, and fresh-session loading. Use
tests/setup-config-home-repro.shwith optional generic--claude-config <dir>/--codex-home <dir>credential sources; keep real HOME and USER unchanged. Do not write unset/empty cases into daily config homes. Run directories can contain credentials and must never be committed or uploaded.Codex is unavailable in this sandbox. Neither pre-fix live reproduction nor final-head live setup/loading evidence is claimed. The operator must supply remaining live evidence and post the exact-head
live-gate; the builder has not posted it. Any changed PR head requires fresh operator proof.The prior
.mergify.yml:queue_rulesfinding is excluded as unrelated to config-home writes; queue behavior is unchanged.