-
Notifications
You must be signed in to change notification settings - Fork 0
compass(rules): holds are the three landing preconditions; other violations are landed and filed (#257) #362
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,12 @@ | |
| names it differently, substitute that name. After a container rebuild | ||
| (`/root` does not survive `teardown.sh`), run | ||
| `git config --global --add safe.directory '*'` and `gh auth setup-git` before | ||
| any git command. A pull as root leaves files | ||
| root-owned, which fails host-side edits silently, so chown after every pull: | ||
| any git command. `setup-git` needs `gh` logged in, and the login is | ||
| `/root/.config/gh/hosts.yml`, which a full teardown also discards; so | ||
| `gh auth login`, or the token file `gpu_docker/CLAUDE.md` names, may be needed | ||
| first (unverified: not yet tested after a real teardown). A pull as root | ||
| leaves files root-owned, which fails host-side edits silently, so chown after | ||
| every pull: | ||
|
|
||
| ``` | ||
| cd <main worktree> && git fetch fork --quiet | ||
|
|
@@ -152,8 +156,11 @@ | |
| per-wave GPU superset. Landing a stacked PR lands every unlanded PR below it, | ||
| so each of those needs the same: its own APPROVE covering its head, and no | ||
| label. A PR whose body declares an escalation without the label | ||
| gets the label. Any other hold names the rule in this file behind it; a rule | ||
| violation seen in an approved PR is filed as an issue, not held. Where a | ||
| gets the label. **Holds are landing preconditions, and there are three:** | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: the closed list scopes Rule quoted, L107: "The label stops all agent action on that issue or PR (no commit, review, amend or merge, even after a passed review)". This line says the holds are exactly three, and the first is " This is narrow. L106 already says to "label each PR it holds", and today no open, unlabelled PR delivers a labelled issue (I checked every open PR's body against the ten open labelled issues). The tip's own landing sentence was also PR-scoped. But the file is read literally, and "there are three" is new. One way to close it is " |
||
| `need human` on the PR or below it (every escalation rule in this file holds | ||
| through this label), an APPROVE covering each head, and the tree check below. A | ||
| hold names the one that is unmet. **A violation of any other rule seen in an | ||
|
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: "there are three" sits beside "Four gates land a task, all required" (L121), and the file never connects them. Rule quoted, L121: "Four gates land a task, all required:". Named result of #257: "an agent reading the file can classify any rule as a hold or a filed violation without history." Read literally, L121 lists four things required to land, and this line lists three landing preconditions. The literal reading still resolves. Gate 4 is the approval (precondition 2). Gate 1 is re-run by the tree check (precondition 3). Gates 2 and 3 are not in the list, so a miss seen in an approved PR is "landed and filed". The PR body gives the reason that is correct: gates 1-3 are what the reviewer checks before the approval exists. But that reason is only in the PR body, which squashes away. Without it, an agent sees "four ... all required" beside "three" and has to reconstruct why. One parenthetical would do it, for example: "(gates 1-3 are checked before the approval that is gate 4; gate 1 is re-checked by the tree check)". This does not change meaning, and it is not required to land. |
||
| approved PR is landed and filed as an issue, not held.** Where a | ||
| handoff note contradicts this file, this file wins. | ||
| - **Before landing on a moved tip, compute the tree that will land:** | ||
| `git merge-tree --write-tree <current tip> <reviewed head>`, adding | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking: the facts cited here check out, and one half of the "unverified" can now be dropped.
Principle 8: "Every claim carries its measurement."
Read-only checks against the running
jgong5_vllmcontainer:gh auth statusgivesLogged in to github.com account jgong5 (/root/.config/gh/hosts.yml). NoGH_TOKENorGITHUB_TOKENis set in the environment, so the login really is that file.docker inspect jgong5_vllmmounts only/data,/md1/users/jgong5 -> /workspace,hf_cache -> /root/.cache/huggingfaceand/mnt.findmnt -T /root/.configresolves to the overlay root, so the file is not on a mount.teardown.shends indocker rm "${CONTAINER_NAME}".setup.shcontains noghstep./workspace/.github-amd-tokenexists on the workspace mount.GH_CONFIG_DIR=<empty scratch dir> GIT_CONFIG_GLOBAL=<scratch file> gh auth setup-gitprintsYou are not logged into any GitHub hosts. Run gh auth login to authenticate.and exits with rc 1 (gh 2.45.0). It wrote nothing, and the real global config is unchanged. So "setup-gitneedsghlogged in" is measured now. What is still unverified is only the teardown itself, and whether the token file is enough.ponytail
shrink:L23-26, about -1 line: "A teardown also discards theghlogin (/root/.config/gh/hosts.yml), andsetup-gitrefuses without one, sogh auth login(or the token filegpu_docker/CLAUDE.mdnames) first; untested after a real teardown."