Fix /cd failing on Windows when target directory contains '.'#323
Closed
wkramme wants to merge 2544 commits intompfaffenberger:mainfrom
Closed
Fix /cd failing on Windows when target directory contains '.'#323wkramme wants to merge 2544 commits intompfaffenberger:mainfrom
wkramme wants to merge 2544 commits intompfaffenberger:mainfrom
Conversation
…-downgrade Fix( Auto-Update): Don't downgrade when local version is newer
…gs-v2 fix: marketplace download/upload response parsing, cache refresh, and unified hash storage
…errors fix(mpfaffenberger#251): Fix pre-existing ruff lint errors and broken DX test imports
Update datasets.py
…-agent slide deck agent completed
…earch Feature/ad group search
…sharing - New tool module: code_puppy/tools/puppy_share_tools.py - puppy_share_upload: push HTML content directly - puppy_share_upload_file: read HTML file from disk and publish - puppy_share_delete: remove a published page (owner only) - puppy_share_list_my_pages: list current user's shared pages - Added sharing URL helpers to urls.py (upload, delete, view, my-pages, svps) - Registered 4 Puppy Share tools in enterprise_tools.py - Defaults to puppy.walmart.com (prod); pass local=True for localhost:8080 - Added comprehensive unit tests
- New SharePuppyAgent registered as walmart-internal agent - Invokable via /agent share-puppy - Has access to all 4 puppy_share tools plus file ops - Defaults to puppy.walmart.com; says 'local/test/dev' for localhost - System prompt guides the agent through upload/delete/list workflows
…login - Updated system prompt to emphasize auth is AUTOMATIC - Agent now calls tools immediately without asking to authenticate - Hardened _get_puppy_token with 3-tier fallback: env var → config module → raw INI parse - Only mentions 'puppy login' if a tool call actually returns a token error
Handle expired puppy_token by triggering auth flow and retrying once for safety validation and telemetry. Adds unit test for 401->reauth->retry behavior.
…401-reauth Fix: re-auth + retry on puppy-backend 401
Restored metaregistry_client.py and the dual-source api_client.py that supports both E2E Open Skills and MetaRegistry BFF marketplaces.
- list_or_search_skills now accepts include_remote=True to query E2E Open Skills and MetaRegistry BFF marketplaces - activate_skill auto-installs skills from remote registries if not found locally - install_skill now injects YAML frontmatter for skills without it - Skills from different sources are tagged with 'source' field - Maintains backward compatibility with local-only defaults
- Revert remote search/auto-install from agent tools (keep local-only) - Sort skills alphabetically in /skill-market TUI (mixed E2E + MetaRegistry) - Pass metadata to install_skill for frontmatter injection - Skills install via /skill-market interaction only
Skills with the same name in multiple directories (e.g., ~/.code_puppy/skills, ~/.claude/skills, ~/.wibey/skills) now only appear once. First directory wins.
- New install_skill_full() downloads ALL files, not just SKILL.md - MetaRegistry skills with scripts (e.g., show-toast, agent-browser) now get their scripts/ directory downloaded - Shell scripts (.sh) are made executable automatically - Shows file count in install status message - Warns if some files failed to download
…load - Switch from stage to prod MetaRegistry URL (https://metaregistry-bff.walmart.com) - Don't create empty scripts/ directory if file downloads fail - Make .ts files executable too (not just .sh) - Note: MetaRegistry /files endpoint currently returns 503, scripts don't download
Windows portable venv build is rock-solid (12-round saga ended with
PR #403 in puppy-frontend wiring installers to consume it), so the
linux phases that were temporarily disabled with
`# DISABLED-FOR-WINDOWS-ITERATION:` are now restored.
Restored:
Phase 1 (python-agent / linux): pip install build deps, uv venv,
bump patch version, install -e ., python_releaseSteps (wheel
build + Artifactory publish), commitVersion (push tag),
slackBuildMessagePython (#code-puppy-pipelines).
Phase 3 (python-agent / linux): write_venv_latest_pointer
(atomically flips gs://puppy-pages/code-puppy-venv/latest/
version.txt so puppy-backend serves the new venv).
Phase 2 (vs2022 windows venv build) is unchanged.
Left disabled (with explanation):
- var(COMMIT_USER): git log -1 --format="%aN"
cmd.exe on vs2022 chokes on the % chars (commit ee1438b history).
Slack message will render "User: " (blank) -- ugly but not broken.
Future fix: move COMMIT_USER assignment INSIDE the python-agent
block so vs2022 never executes it.
First manual_release run after restoring the linux phases blew up at
`uv build` on vs2022 with:
Failed to fetch:
https://pypi.ci.artifacts.walmart.com/.../hatchling/
Request failed after 3 retries
tunnel error: failed to create underlying connection
tcp connect error
The 'tunnel' wording is the giveaway -- vs2022 agents auto-pick up a
corporate HTTPS proxy via WPAD/PAC, and `uv` (a Rust binary) honors
HTTPS_PROXY/HTTP_PROXY verbatim. It does NOT auto-bypass internal
hosts the way Windows-native HTTP libraries do, so every `uv` request
to pypi.ci.artifacts.walmart.com gets CONNECT-tunneled through
sysproxy and dies on the inner TCP connect.
The previous 12 manual_release iterations were only green because
`hatchling` had been cached locally on whichever sticky vs2022 agent
we kept landing on. Looper rotated us to a fresh agent today and the
empty cache exposed the latent bug.
Fix: set NO_PROXY (and lower-case no_proxy -- Rust HTTP stacks are
inconsistent about which they honor) so *.walmart.com / *.wal-mart.com
traffic skips sysproxy and hits artifactory directly.
Smallest possible patch -- 17 lines, all in the existing prereqs
section before `uv build` runs. No agent config or proxy infra
changes needed.
Phase 3 of manual_release blew up at line 1 with:
/tmp/script6495653384719697344.sh: 1: set: Illegal option -o pipefail
Looper's python-agent docker image uses /bin/sh -> dash (Debian
default), not bash. `set -o pipefail` is a bash-ism. The other
working shell block in this file (backfill_artifactory) doesn't use
pipefail at all, which is why it never tripped this.
Switch to `set -eu`. There are no critical pipe chains in this
script -- the only pipeline is grep|head|sed for the pyproject.toml
fallback, and that's already guarded by the next `if [ -z "$VERSION" ]`
check, so a silent pipeline failure couldn't slip a bad version through.
Phases 1 & 2 of this manual_release run were green (Windows venv was
built + uploaded successfully) -- this is the final hop before the
`latest` pointer flips. Once green, /code-puppy-venv/version starts
returning real data instead of stale/404.
Previous run got past hatchling (NO_PROXY=walmart.com worked!) but
died at `uv pip install` of the ripgrep wheel. Artifactory issued a
302 redirect to:
https://700ee0a34astg.blob.core.windows.net/main-ci-artifacts-...
uv followed the redirect, tried to CONNECT-tunnel through sysproxy,
and died with:
tunnel error: failed to create underlying connection
tcp connect error
os error 10060 (WSAETIMEDOUT)
That blob storage account is Walmart-Azure on the ExpressRoute --
internal-routed, must NOT go through sysproxy. But it's on a
public-looking *.blob.core.windows.net hostname, so the previous
walmart.com-only NO_PROXY didn't catch it.
Going belt-and-suspenders this time:
1. Strip HTTP_PROXY / HTTPS_PROXY / ALL_PROXY (and lower-case
variants) from this process entirely. Every URL this script
touches (puppy-backend, artifactory, blob storage, GCS upload)
is internal-routed -- no proxy needed for ANY of them. Root-cause
fix: uv doesn't even SEE a proxy var to honor.
2. Keep NO_PROXY as a backup allowlist, expanded to include
*.blob.core.windows.net (artifactory wheel redirects) and
*.googleapis.com (GCS upload in step 4). Catches the case where
a uv subprocess re-injects a proxy from the user profile or
registry.
This is the FINAL hop -- once Phase 2 is green, Phase 3 (the
write_venv_latest_pointer dash-pipefail fix landed in 4ba6cb0)
will fire and the latest pointer will finally flip.
Phase 1 (linux wheel publish) was already green this round.
Build mpfaffenberger#68 caught three bugs in Phase 3 plus exposed a real data-integrity hazard: BUG 1 (data integrity, scariest): VERSION resolution went through: python -c "import code_puppy; print(code_puppy.__version__)" This docker agent has a fresh checkout with no editable install, so importlib.metadata.version("code-puppy") raises and the except branch in code_puppy/__init__.py returns the literal string "0.0.0-dev". The script blindly accepted it and was 1 step away from writing "0.0.0-dev" to the latest pointer -- which would have bricked every Code Puppy installer in prod the moment the next download happened. Yikes. Fix: kill the import attempt entirely. pyproject.toml is the only source of truth (Phase 1 already bumped it via `uv version --bump patch` and committed it back to the repo). BUG 2 (immediate failure): pip install --allow-insecure-host pypi.ci.artifacts.walmart.com ... -> 'no such option: --allow-insecure-host' That's a uv-pip-only flag. Real pip uses --trusted-host. BUG 3 (defense in depth): No semver sanity check before flipping the latest pointer. Even with bug 1 fixed, a future regression that landed a malformed version in pyproject.toml would silently break prod. Fix: two-step dash-portable case validation: Step 1: reject any character that isn't a digit or dot (catches '0.0.0-dev', 'v0.0.499', 'main', empty, etc.) Step 2: require at least the X.Y.Z shape Tested locally against 9 inputs: ACCEPT: '0.0.499', '1.2.3.4' REFUSE: '0.0.0-dev', '', 'main', 'v0.0.499', '0.0.499 ', '0.0', '0.0.499.dev0' First draft of the regex used only a single `case` glob '[0-9]*.[0-9]*.[0-9]*' -- but dash globs aren't real regex and '*' greedily ate '-dev', so '0.0.0-dev' would have ACCEPTED. Caught by the local test loop before pushing -- this is exactly the kind of silent failure the gate was supposed to prevent.
Build mpfaffenberger#69 got further than ever -- VERSION resolved correctly to 0.0.500, sanity gate passed, pip install ran without flag errors -- but then the inline python3 heredoc died with: ModuleNotFoundError: No module named 'requests' Smoking gun in the same log: pip's update notice said 'To update, run: python3.12 -m pip install --upgrade pip' So bare `pip` is python3.12's pip, but bare `python3` resolves to a different interpreter on this docker image (probably python3.10 from the system). Install 'succeeded' in a suspicious 3 seconds because google-auth/requests were already cached for python3.12, but python3 (3.10) saw an empty site-packages and crashed. Fix: use `python3 -m pip install ...` so the install is guaranteed to land in the same interpreter that runs the heredoc. Same trick we already use everywhere else in the codebase whenever multiple Pythons coexist on PATH.
Build mpfaffenberger#70 got past every previous bug (VERSION resolved, gate passed, deps installed correctly into the right interpreter) but died at GCS_SA_KEY_B64 decode with: binascii.Error: Incorrect padding Phase 3 has actually never run successfully -- it was added in 7d9faa9, immediately disabled in 50116ba for the Windows iteration, and only restored in 0bfb0c5 last week. Meanwhile, the Windows side spent ~12 rounds of its own beating on the EXACT same secret with the EXACT same gotchas. Stealing all that hard-won knowledge instead of re-discovering it. Three Looper-secret-pipeline behaviors we need to defend against (all documented in scripts/upload_venv_to_gcs.ps1, lines 47-77): 1. CR/LF/spaces injected mid-string -- strip ALL whitespace first (re.sub(r'\s+', '', raw)) 2. ENC[...] decryption is INCONSISTENT across agent types: linux python-agent gives us still-base64, Windows vs2022 decodes one extra hop and gives raw JSON. Sniff the first non-whitespace char: '{' => already JSON, no b64 decode. Defensive even on linux because Looper's behavior may shift between releases. 3. Trailing '=' padding stripped somewhere along the way -- re-pad to a multiple of 4 before decoding. THIS is what bit build mpfaffenberger#70 ('Incorrect padding'). Tested locally against 0/1/2 missing-pad cases, all round-trip cleanly. Also added safe diagnostic output (length + first-8 + last-8 chars + length mod 4) -- if this fails again we'll know exactly which of the three failure modes hit, without leaking the secret. After the env-var-decode passes, also validates client_email is present in the parsed JSON before attempting GCS auth. Fails fast with a helpful message rather than a confusing google-auth stack trace if the key is structurally bogus.
Build mpfaffenberger#71 finally got past secret decode AND service-account loading (client_email logged as svc-gi-etl@wmt-ww-gg-gi-dev.iam.gserviceaccount.com) but then crashed in cryptography's PEM parser: ValueError: Unable to load PEM file. ... MalformedFraming Mike spotted it: it's the PEM `\n`-vs-real-newline gotcha. Looper's secret pipeline JSON-stringifies the SA blob, and the `\n` escapes inside the "private_key" value survive as the literal 2-character sequence `\` + `n` instead of being un-escaped into actual newline bytes. The cryptography lib then sees: -----BEGIN PRIVATE KEY-----\nMIIE...\n-----END PRIVATE KEY-----\n as a SINGLE LINE, which violates PEM framing rules. Fix: detect-and-repair, NOT blanket replace. - If private_key starts with `-----BEGIN` AND has no real newline anywhere, the escapes need un-doing. .replace('\\n','\n') restores them. - If it starts with `-----BEGIN` AND already has real newlines, leave it alone (no double-decoding). - Anything else: fail loud with diagnostics rather than letting a bad key reach the crypto layer. Tested locally: - Mangled (3-line PEM as 1 line with literal \n) -> repaired to 3 lines ✅ - Already-correct (real newlines) -> left as-is ✅ Diagnostic output added (DIAG: lines) so future failures are debuggable from logs alone. This is the same class of bug the Windows side had to learn the hard way during its 12-round saga -- specifically the lesson behind ee1438b / 6207775 territory. Linux Phase 3 is now caught up.
Build mpfaffenberger#72 fired the 'already has real newlines; leaving as-is' diagnostic and STILL failed with MalformedFraming. My binary 'has-newline-yes-or-no' heuristic was too weak -- a key with a trailing real \n + literal \n separators in the middle slips through the check but is still structurally broken (-----BEGIN line + collapsed body + -----END line, all on effectively one logical line to the cryptography parser). Three changes: 1. Unconditional un-escape: if ANY '\\n' literals appear, replace them with real newlines. Idempotent on already-good keys ('\\n' won't match '\n'), safe to always run. 2. CRLF -> LF normalization (cryptography is usually tolerant but belt + suspenders). 3. Dump head40 + tail40 + (real_nl, esc_nl, cr) counts BEFORE the repair so we can see exactly what Looper handed us. The 40-byte window is enough to see '-----BEGIN PRIVATE KEY-----' framing and trailing whitespace without revealing key bytes. 4. Final structural sanity: must start with -----BEGIN, must contain -----END, must have at least 3 newlines after repair. Fail loud with a useful message rather than letting a malformed key reach load_pem_private_key for the third time in a row. Tested locally against 4 input variants: - collapsed (literal \\n only): -> 4-line PEM after repair ✅ - already-good (real \\n only): -> 4-line PEM, no-op ✅ - mixed (literal + 1 real): -> 4-line PEM after repair ✅ - crlf (\\r\\n): -> 4-line PEM after repair ✅
…D markers) Build mpfaffenberger#73 diagnostics finally exposed the real bug -- Mike spotted it in the head/tail dump: DIAG: private_key head40='-----BEGINPRIVATEKEY-----\\nMIIEvQIBADANBg' DIAG: private_key tail40='I8HPnUZTz8hi58=\\n-----ENDPRIVATEKEY-----\\n' The SPACES BETWEEN 'BEGIN', 'PRIVATE', 'KEY' (and 'END', 'PRIVATE', 'KEY') are GONE. Looper's secret pipeline strips internal whitespace from values somewhere along the way. cryptography rejects this with MalformedFraming because PEM requires exactly: -----BEGIN PRIVATE KEY----- (with single spaces) Phase 2 (Windows venv upload) consumes the EXACT same secret and works fine because it sidesteps PEM parsing entirely -- signs the JWT manually with raw CngKey.Import() on the base64-decoded body bytes (scripts/upload_venv_to_gcs.ps1, lines 108-128, the exact lesson behind commit 6207775 which Mike was just remembering). Easiest port of that strategy to Python: don't try to fix the PEM in-place. Strip the mangled markers + whitespace, base64-decode the body to raw PKCS8 DER, then *reconstruct* a textbook-clean PEM (proper marker spacing, 64-char line wrap) before handing it to google-auth. cryptography sees a perfect PEM regardless of what Looper mangled. Repair pipeline (5 deterministic steps): 1. Un-escape '\\n' -> '\n' (idempotent on already-good keys) 2. Normalize CRLF -> LF 3. Strip BEGIN/END marker LINES via -{2,}[^-]*BEGIN[^-]*-{2,} (catches mangled 'BEGINPRIVATEKEY', proper 'BEGIN PRIVATE KEY', 'BEGIN RSA PRIVATE KEY', etc.) 4. Strip ALL whitespace from what's left -> pure base64 5. Re-pad to mult-of-4, b64decode -> raw PKCS8 DER bytes 6. Re-encode + textwrap to 64 cols + slap on textbook markers Tested locally end-to-end with cryptography library: - Generated real 2048-bit RSA key - Mangled it the EXACT same way Looper does ('BEGINPRIVATEKEY' / 'ENDPRIVATEKEY' with literal \\n separators) - Verified cryptography rightly rejects the mangled version - Ran our repair pipeline - serialization.load_pem_private_key() succeeds on rebuilt PEM - Rebuilt key matches original byte-for-byte (private_numbers comparison) Diagnostic output retained from previous attempt -- if this STILL fails, the new diagnostics will tell us ex what we missed.
PROBLEM:
The puppy-frontend release-management UI shows nothing newer than
0.0.461. Pyproject.toml is at 0.0.504. The whole 0.0.462 -> 0.0.504
range never made it to genaica-generic-prod-local (which the frontend
queries via /api/storage/genaica-generic-prod-local/code-puppy/).
ROOT CAUSE:
python_releaseSteps was using the SAME unreliable version-detection
trick that almost bricked Phase 3 yesterday:
VERSION=$(uv --native-tls run python -c \
"import code_puppy; print(code_puppy.__version__)")
When importlib.metadata.version('code-puppy') raises -- e.g. because
uv's editable-install caching layer didn't refresh after the
`uv version --bump patch` step -- code_puppy/__init__.py's except
branch returns the literal string '0.0.0-dev'. Curl then:
PUT ${REPO}/code-puppy/0.0.0-dev/code_puppy-<real_ver>-*.whl
Artifactory rejects with 4xx because the path mismatch (or possibly
because /code-puppy/0.0.0-dev/ doesn't exist with the right perms).
`curl --fail` returns non-zero, but the surrounding shell had no
`set -e`, so the for-loop just kept going and the macro 'succeeded'.
Slack post lit up green. Nobody noticed.
The 0.0.439 -> 0.0.458 -> 0.0.461 timestamp gaps in the generic repo
(2026-04-01 / 2026-04-17 / 2026-04-17) confirm this has been
intermittently broken since mid-April.
FIX (mirrors what we did for Phase 3 yesterday):
1. Add `set -eu` so curl --fail actually aborts the build.
2. Read VERSION from pyproject.toml (single source of truth) instead
of trusting importlib.metadata.
3. Hard semver gate (two-step dash-portable case validation, same
as write_venv_latest_pointer) -- refuse to upload anything that
isn't X.Y.Z. Catches '0.0.0-dev', empty, 'main', etc.
4. Cross-check that the wheel `uv build` produced has a filename
matching VERSION -- catches stale-build / wrong-pyproject cases.
5. Dump dist/ ls before upload so future failures are debuggable
from the build log alone.
NOT IN SCOPE (separate task):
Backfilling 0.0.462 -> 0.0.504 into genaica-generic-prod-local --
these wheels DO exist on Artifactory PyPI (external-pypi mirror)
because twine handled that part. Need to copy them across with
scripts/backfill_gh_to_artifactory.py adapted for PyPI->generic,
or just re-run manual_release a few times to bump forward.
Disregards the wash of 0.0.497 -> 0.0.506 versions accumulated during the multi-day Phase 1/2/3 manual_release debugging saga (commits 0bfb0c5 through 719c585). Those bumps mostly went nowhere useful: - genaica-generic-prod-local has nothing past 0.0.461 (the bug fixed in 719c585 -- versions silently uploaded to /0.0.0-dev/ instead of the real version path). - GCS code-puppy-venv only has the Windows zips that survived Phase 2 (a couple iterations). - Artifactory PyPI external-pypi probably has all of them but nobody consumes that for the release-management UI. 0.0.496 was the version in pyproject.toml at commit 0bfb0c5^ (parent of the 're-enable wheel publish' commit that kicked off the saga). Resetting to that means: - Next `manual_release` run will bump-patch to 0.0.497, which becomes the FIRST clean version with all the fixes applied: * python_releaseSteps reads version from pyproject.toml * curl --fail + set -eu actually aborts on upload failure * Phase 3 latest-pointer flip works (PEM rebuild) * Frontend release-management UI will show new versions again uv.lock was stale at 0.0.488 (Linux phase had been disabled during the Windows iteration so commitVersion never refreshed it). Updated to match the new pyproject.toml -- uv will regenerate cleanly on next install.
After 500+ patch-level releases (and a recent string of broken-CI bumps
that piled up dirty 0.0.497 -> 0.0.506 wheels in artifactory), it's
time to leave the 0.0.x era behind.
Setting pyproject.toml + uv.lock to 0.1.0 directly. The next
`manual_release` run will `uv version --bump patch` -> 0.1.1, which
becomes the FIRST published version in the 0.1.x line. (0.1.0 itself
is just the pre-bump sentinel and won't ever be published as a wheel.)
Why not yank the 0.0.497 -> 0.0.506 dirty wheels first?
- Artifactory generic-prod-local supports DELETE via REST, but with
the version cliff we're about to create, those wheels become
silent tombstones nobody will ever request. Yanking is busywork.
Adjacent fix already in flight (puppy-frontend):
feat/windows-installer-fast-venv-path branch, commit 23196dd,
flips artifactory_client.py from genaica-generic-prod-local to
ai-innovation-lab-generic-prod-local. Once that PR merges + this
release ships, the release-management UI will show 0.1.1 as the
newest promotable version.
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
3 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
Users reported that
/cdcan fail on Windows when the target directory contains a dot (for example:repo.v2).This PR fixes that behavior.
User-facing issue
On Windows, commands like the following could fail with
Not a directoryeven when the directory exists:/cd C:\Users\alice\repo.v2/cd "C:\Users\alice\repo.v2"Root cause
handle_cd_command()used POSIX-styleshlex.split()for all platforms.On Windows, that parsing can corrupt native backslash paths, which then causes directory checks to fail.
Dotted folder names were a common real-world case where this surfaced.
What changed
code_puppy/command_line/core_commands.pyshlexparsing on Windows (os.name == "nt") to preserve Windows path semantics.split(maxsplit=1)) for safer handling.tests/command_line/test_core_commands_extended.pyAdded regression tests for Windows
/cdwith dotted directory names:C:\...\repo.v2)"C:\...\repo.v2")Validation
uv run pytest -q tests/command_line/test_core_commands_extended.py -k "cd_windows or cd_with_special_characters or cd_with_relative_path or cd_with_tilde_expansion"uv run pytest -q tests/test_command_handler.py -k "test_cd_"Risk
Low.
The change is scoped to
/cdargument parsing and includes focused regression coverage.