Skip to content

fix(deploy): ignore Python bytecode downstream - #435

Merged
KbWen merged 7 commits into
KbWen:mainfrom
zerone0x:fix/downstream-ignore-python-bytecode
Sep 14, 2026
Merged

KbWen merged 7 commits into
KbWen:mainfrom
zerone0x:fix/downstream-ignore-python-bytecode

Conversation

@zerone0x

@zerone0x zerone0x commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Maintainer update (2026-09-14)

Taken over after the review went unanswered for a week. The comment below has the rationale and evidence. The PR now does something different from the original summary further down:

  • It adds one framework-scoped line, .agentcortex/**/__pycache__/, to the managed downstream .gitignore block, instead of repo-wide __pycache__/ + *.pyc. The line is registered in managed[] and written last in the block.
  • Tests:
    • every emitted block entry must be strippable
    • every deployed .py must sit under the scoped pattern
    • a real double deploy must leave .gitignore byte-identical and must not ignore the adopter's own bytecode
  • Records: backlog [Bug] deploy.sh fails path resolution when executed from subdirectories #191 is marked Shipped and there is an SSoT Ship History entry. The work log is archived.

Closes #430


Original submission:

Summary

Classification

  • tiny-fix (typo, rename, config)
  • quick-win (1-2 modules, clear scope)
  • feature (new workflow / skill / capability)
  • hotfix (urgent fix for broken behavior)
  • architecture-change (cross-module structural change)

Evidence

$ uvx pytest tests/ci/test_deploy_tiering.py -q
.........ss.............................                                 [100%]
38 passed, 2 skipped in 9.93s

$ bash .agentcortex/bin/validate.sh
Summary: pass=99 warn=4 fail=0 skip=3
Agentic OS integrity check passed

Checklist

  • validate.sh passes locally
  • No new files added to root unless absolutely necessary
  • All internal markdown links resolve correctly
  • CHANGELOG.md updated (for feature / architecture-change)

@zerone0x
zerone0x requested a review from KbWen as a code owner September 5, 2026 20:04
@KbWen

KbWen commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Thank you for this — and welcome. This is a real defect, the diagnosis in #430 is accurate, and you picked the right shape of fix: additive, no design question attached, with a regression test alongside it. Your evidence reproduces on my machine too.

There is one thing to change before it can merge, and it is not something your evidence could have surfaced.

The managed ignore block is written in two places

deploy.sh emits the block in write_downstream_ignore_block(), which your patch updates. But it also has to remove the previous copy on a re-deploy, and that half lives in the managed[] table inside strip_managed_ignore_blocks() (.agentcortex/bin/deploy.sh:1114). The awk loop drops a line only while every line it sees is a known managed entry — the moment it meets one that is not in the table, it sets skip = 0 and prints the rest of the block verbatim:

skip {
    if ($0 == "" || ($0 in managed) || $0 ~ /^#/) { next }
    skip = 0
}

__pycache__/ is not in that table, so from the second deploy onward the strip stops right at your new lines and everything after them survives — outside the # Agentic OS Template markers, where no later deploy can ever reclaim it.

Reproduction

Three real deploy.sh runs into a fresh git repo (not a fixture), counting .gitignore lines:

deploy origin/main (control) this PR this PR + the two managed[] lines
#1 32 36 36
#2 32 50 36
#3 32 64 36

At deploy #3 the adopter's .gitignore holds three copies each of __pycache__/, *.pyc, .claude/settings.local.json, .openrouter/, .claude-chat/, .cursor/ and .antigravity/scratch/, growing by 14 lines every time. main is idempotent today, so this would be a regression introduced by the fix.

Suggested change

Two lines, next to their sibling in the same table:

        managed["*.acx-local"] = 1
+       managed["__pycache__/"] = 1
+       managed["*.pyc"] = 1

This is the same two-place edit #173 made when *.acx-local was added, so there is a precedent in the file's history to match.

Why your green run did not catch it

Please don't read the above as a process failing on your side — the repo did not give you a way to see it:

  • test_deploy_gitignores_python_bytecode greps deploy.sh as one string, so it matches the heredoc and would equally match the managed[] table. That is exactly the shape of its sibling test_deploy_gitignores_acx_local_sidecar, so you followed the local convention correctly.
  • Nothing in tests/ci/test_deploy_tiering.py deploys twice, so re-deploy idempotency is not covered at all.
  • validate.sh / validate.ps1 check the emitted block against a list of required patterns, not an exhaustive one, so they stay green either way.

If you are willing, a test that runs _deploy(target) twice and asserts the .gitignore line count is unchanged would close that gap for every future entry, not just these two. Entirely optional — the two managed[] lines are the blocker; I am happy to merge with just those and file the test separately.

One thing you can skip

#430 asks for parity in the PowerShell path. That is already satisfied: deploy.ps1 resolves a real Git Bash and delegates to deploy.sh, so it carries no ignore block of its own. Nothing to do there.

CI has not run yet because workflow runs from a fork need maintainer approval. I have read the full diff (16 lines across two files, nothing else in it), so there is no hold-up on that side.

Thanks again for taking the time to send this properly — issue reference, classification, evidence and a test. It is a genuinely good first contribution and I would like to merge it.

@KbWen

KbWen commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Quick follow-up: I approved the workflow runs, and CI came back fully green — 18 passed, 1 skipped, 0 failed, including Deploy Smoke Test, Deploy Smoke Test (No Python) and CI Structural Tests.

Please don't read that as "ready to merge" — it is the coverage gap from my previous comment, now demonstrated rather than argued. Eighteen green checks, and not one of them deploys twice, so the .gitignore growth is invisible to all of them. The two managed[] lines are still the blocker.

I mention it because a green CI on your own PR is a reasonable thing to trust, and here it would have misled you through no fault of yours.

KbWen and others added 6 commits September 14, 2026 10:00
…y idempotent (KbWen#430)

Completes KbWen#435. The submitted patterns were not in the managed[] strip
table, so every re-deploy left the old block outside the markers and the
adopter's .gitignore grew by 14 lines (measured 33 -> 47 -> 61).

- Emit `.agentcortex/**/__pycache__/` instead of repo-wide `__pycache__/`
  and `*.pyc`: all deployed .py files live under .agentcortex/, and a
  repo-wide rule would also ignore the adopter's own bytecode.
- Add the entry to managed[] so the block is replaced, not duplicated.
- Tests: every emitted entry must be strippable; every deployed .py must
  sit under the scoped pattern; a real double deploy must leave
  .gitignore byte-identical, keep adopter rules, ignore framework
  bytecode and not the adopter's.
- Backlog KbWen#191 -> In Progress.

Co-authored-by: zerone0x <39543393+zerone0x@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…host git config (KbWen#430)

Review follow-ups on KbWen#435:

- An older deploy.sh stops stripping at the first block entry it does not
  know, leaving every later block line outside the markers. With the new
  entry mid-block, alternating main/branch deploys grew .gitignore by
  10-13 lines per round (measured 36 -> 46 -> 49 -> 59 -> 62 -> 72).
  Placed last, a downgrade leaves one stray line; alternation adds 3
  lines per round (36 -> 36 -> 39 -> 39 -> 42). The managed[] table now
  says to add future entries last.
- The double-deploy test ran `git check-ignore` under the developer's
  global excludes file, so a machine ignoring __pycache__/ globally
  failed it on correct code. It now passes an empty core.excludesFile.

Co-authored-by: zerone0x <39543393+zerone0x@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…Wen#430)

`Path.write_text(newline=)` is 3.10+; test_write_text_newline_ratchet
flagged it in the full suite. Write the adopter fixture as bytes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SSoT Ship History entry (rotated at cap 10), heartbeat 170 -> 171,
backlog KbWen#191 Shipped with the row rewritten to what actually shipped,
Work Log archived and chained into INDEX.jsonl.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@KbWen

KbWen commented Sep 14, 2026

Copy link
Copy Markdown
Owner

Taking this over to land it

@zerone0x, there has been no update for a week since the review. As offered then, I have finished the change on your branch rather than letting it stall. Your commit stays in the history, and the new commit credits you as co-author. Thank you again: the issue reference, classification and test made this easy to pick up.

Nothing was force-pushed. I merged main into the branch and added commits on top.

What changed from the submitted version, and why

  1. The blocker from the review: the new entry is now in the managed[] strip table. Without it, every re-deploy grew the adopter's .gitignore by 14 lines.
  2. The pattern is scoped to the framework: .agentcortex/**/__pycache__/ instead of repo-wide __pycache__/ + *.pyc.
    • All 19 .py files that deploy ships live in .agentcortex/tools/.
    • A repo-wide rule would also ignore the adopter's own bytecode anywhere in their project. That is a silent policy change caused by installing the framework, and it duplicates a rule most Python projects already have.
    • Python 3 writes bytecode only into __pycache__/, so *.pyc adds nothing for framework files.
  3. The tests now check the behaviour instead of grepping the whole file:
    • test_deploy_ignore_block_entries_are_all_strippable: parses the emitted block and the managed[] table separately. Every emitted entry must be strippable, which covers future entries too.
    • test_deploy_ignores_framework_bytecode_in_framework_namespace_only: the scoped pattern must be present, the repo-wide ones absent, and every .py in the deploy golden must sit under .agentcortex/.
    • test_redeploy_leaves_gitignore_unchanged_and_adopter_policy_alone: deploys twice into a real git repo. The .gitignore must be byte-identical, the adopter's own rules must be kept, git check-ignore must ignore framework bytecode, and it must not ignore the adopter's.

Evidence

Three real deploy.sh runs per target, into fresh git repos. The Python-adopter target already has its own __pycache__/ and *.pyc rules.

origin/main as submitted this PR now
fresh target, .gitignore lines after deploy 1 / 2 / 3 29 / 29 / 29 33 / 47 / 61 33 / 33 / 33
Python adopter, lines after deploy 1 / 2 / 3 35 / 35 / 35 39 / 53 / 67 39 / 39 / 39
adopter's own lines preserved yes no yes
framework .pyc visible to git status after validate.sh (fresh target) 2 0 0
adopter's own app/legacy.pyc ignored (fresh target, no rule of their own) no yes no

Mutation checks (each invariant broken on purpose, then restored byte-identical):

  • Dropping the managed[] entry fails the strippable test and the double-deploy test.
  • Restoring the repo-wide pair (even with managed[] entries) fails the namespace test and the double-deploy test.
  • Adding a deployed .py outside .agentcortex/ to the golden fails the namespace test.

An independent review pass found two more issues. I reproduced both before fixing them:

  • The double-deploy test was reading the developer's global git excludes. On a machine that ignores __pycache__/ globally it failed on correct code. It now passes an empty core.excludesFile.
  • Mixed framework versions deploying into one repository. An older deploy.sh stops stripping at the first block entry it does not know, so everything after that entry stays behind. With the new line in the middle of the block, alternating main/branch deploys grew the file by 10–13 lines per round. The entry is now written last (the managed[] table says so for future entries):
    • A single downgrade now leaves one stray line.
    • Alternating versions adds 3 lines per round.
    • The remainder lives in code that is already deployed, so it is recorded here rather than claimed fixed.

Also run against the real downstream flows, each compared with origin/main:

  • upgrading an install that already committed bytecode
  • the legacy AI Brain OS block
  • a CRLF .gitignore
  • a subdirectory install inside a monorepo
  • --no-python
  • deploy.ps1 vs deploy.sh, which produce byte-identical output

Validator tallies are unchanged in every case.

Test runs:

  • Full CI-equivalent suite locally (951 tests): 949 passed, 1 skipped. The 1 failure was mine: the new test used write_text(newline=), which is 3.10+, and the 3.9-floor ratchet caught it. That is fixed, and the ratchet plus test_deploy_tiering.py then passed (43 passed, 1 skipped).
  • Both validators: exit 0, pass=99 warn=4 fail=0 skip=3, identical. All 4 warnings were already present.

For existing adopters

Anyone who already committed .agentcortex/tools/__pycache__/ keeps it tracked, because .gitignore does not untrack files. Deploy deliberately does not run git rm inside an adopter's repository. The one-time cleanup belongs in the release notes:

git rm -r --cached .agentcortex/tools/__pycache__

Closes #430.

@KbWen
KbWen merged commit 7bc4955 into KbWen:main Sep 14, 2026
19 checks passed
@KbWen KbWen mentioned this pull request Sep 14, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deployed .gitignore omits __pycache__/ and *.pyc, so the framework's own tools dirty the adopter's tree

2 participants