Skip to content

install.sh and provision resolve "the login profile" differently, so their dedupe marks can miss each other #191

Closed

Description

@blooop

Question

install.sh and the provision script answer "which file is the login profile?" differently, so their dedupe marks can land in different files.

devlaunch/tools.py:97-99 resolves the file bash will actually read — .bash_profile → .bash_login → .profile, first that exists — precisely because appending to ~/.profile in an image that ships a ~/.bash_profile writes to a file bash never sources. .devcontainer/claude-code/install.sh:100 hardcodes PROFILE="$TARGET_HOME/.profile" and then appends the same two # devlaunch:-marked lines (install.sh:101-102).

On an image shipping ~/.bash_profile, the two writers therefore dedupe against different files: install.sh's grep -qxF looks in .profile, provision's looks in .bash_profile, and cross-writer recognition silently fails. Today the cost is a duplicate PATH prepend (benign — PATH dedupe is idempotent in effect), and worse, install.sh's lines land in a file that image's bash never reads at all, so the feature's PATH setup is dead on exactly those images. Latent-not-live on this repo's own devcontainer (no .bash_profile in the base image), which is why it has not bitten.

Two smaller findings from the same review (devlaunch#189), fix in the same pass:

  • test/unit/test_tools.py:200 filters profile-append lines with '>> "$PROFILE"' in line or '>> "$profile"' in line. Nothing anywhere writes to a lowercase $profile; the dead arm can only ever silently admit a future writer that dodges the landing test's assumptions.
  • install.sh:100 sets PROFILE= without local inside install_claude_code(), breaking the file's own local idiom (install.sh:106-107,162) and leaking the variable into main's scope.

The natural shape: install.sh adopts the same three-way resolution (the four lines from tools.py:97-99, verbatim, with $TARGET_HOME for $HOME), and the landing test extends to pin that both writers resolve identically — the relation written once or asserted equal, so it cannot drift again.

Found by devlaunch#189's review (PR, approved with notes); recorded there and never ticketed.

Activity

  1. self-assigned this
    on Aug 15, 2026
  2. blooop commented on Aug 15, 2026

    @blooop
    OwnerAuthor

    breadcrumb: building this ticket. What I will build, restated: the devcontainer feature installer currently declares the login profile to be .profile unconditionally, while the provision and transfer scripts resolve it the way bash does — first existing of .bash_profile, .bash_login, .profile. On an image shipping a .bash_profile the installer therefore writes its marked lines into a file bash never sources, and neither writer can see the other's marks. The installer adopts the same resolution, expressed once in the module and pasted into the installer the way its two append fragments already are.

    Seams I intend to test at, all pre-agreed by the ticket:

    1. Which file each writer resolves, run for real against bash's own answer. The resolution block of each of the three scripts (provision, transfer, feature installer) is executed in a scratch home, and the file it names is compared against the file a real bash -l actually sources in that same home — across every shape of home (.bash_profile present, .bash_login present, both, neither). This pins the ticket's claim directly rather than by text: an oracle bash gives, not one the test recomputes. It also makes "both writers resolve identically" a consequence rather than a separate assertion.
    2. The installer carries the module's rendered resolution verbatim, the same way its two append fragments are already pinned byte-for-byte — the relation lives in one place and the installer is asserted equal to it, so a change to one side fails loudly instead of drifting.
    3. The landing test's own line filter. Its dead lowercase-$profile arm goes; the filter is pinned to match what the scripts actually write, so a future writer cannot dodge the landing test by naming its variable differently and pass silently.

    Aligning with the # devlaunch: marked-line convention from #164/#206 rather than introducing any second marking scheme: the marks and their content-hash derivation are untouched, and only the question of which file they are written into changes.

    Small third finding from the ticket folded into the same pass: PROFILE becomes local in install_claude_code(), matching the file's own idiom.

  3. blooop commented on Aug 15, 2026

    @blooop
    OwnerAuthor

    breadcrumb: built and pushed — PR #213. The devcontainer feature's installer now resolves the login profile the way bash does instead of naming .profile, so its two marked lines land in the file a login shell actually reads and in the same file the provision script guards against. The resolution is rendered from one place and pasted into the installer with $TARGET_HOME for $HOME — the only difference the writers are allowed — following the existing paste-and-pin arrangement for the marked append lines rather than adding any second scheme.

    Both smaller findings are in the same commit: PROFILE is local (checked under dash, which is what #!/bin/sh resolves to on the base image — it no longer leaks into main), and the landing test's dead lowercase-$profile arm is gone, replaced by a test that every append in these scripts goes to $PROFILE so the filter is provably complete rather than merely permissive.

    The load-bearing test runs each writer's edit in a scratch home of every shape and then asks a real login bash what its PATH is, so bash is the authority rather than a rule the test restates — and all three writers agreeing is a consequence of each being right instead of a separate assertion. It was red for all 7 feature cases before the fix, and six mutations (each arm of both resolutions, drift in either direction, and an append hiding under another name) each turn exactly the expected cases red.

    pixi run ci exits 0: 1686 passed, pylint 10.00/10.

  4. blooop commented on Aug 15, 2026

    @blooop
    OwnerAuthor

    breadcrumb: PR #213 checks all green — ci on default/py310/py311/py312/py313, e2e, prek, both codecov gates, and the coverage gate job. Handing off to review; nothing outstanding from the build side.

  5. blooop commented on Aug 15, 2026

    @blooop
    OwnerAuthor

    wf-review verdict on #213: Approve — both axes clean; Spec axis re-ran the headline mutations (exactly the predicted cases red) and verified the equality oracle compares against the module rendering, not the script's own text. Riders recorded in the review (delete the superseded text-shape test, HOME_SHAPES comment, >> filter). Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions