Skip to content

feat: DOTBOT_ACT_MODEL for the act model, plus the fixes that unblock dotbot approval - #7

Merged
wezell merged 7 commits into
mainfrom
feat/dotbot-model-variables
Sep 29, 2026
Merged

wezell merged 7 commits into
mainfrom
feat/dotbot-model-variables

Conversation

@wezell

@wezell wezell commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

What / why

Started as "act model should come from an org/repo variable like the review model already does", and turned into a chain of fixes uncovered while trying to get this very PR approved. 6 commits.

1. DOTBOT_ACT_MODEL drives the act model (207b094, 4fadf9f)

Review already took its model from DOTBOT_REVIEW_MODELS (first entry = primary reviewer, the rest = the "fight" roster); act had no equivalent. Now:

Variable Mode Effect
DOTBOT_REVIEW_MODELS review Comma-separated roster → primary + extras (pre-existing)
DOTBOT_ACT_MODEL act Single slug for /dotbot edits
  • Replacement, not addition: set → authoritative; unset → in-repo .openrouter-review.yml / action default.
  • Rendered to $RUNNER_TEMP/dotbot-act-model.yml and passed via config_path, never written into the checkout — act mode pushes to the PR branch, so a generated file in the worktree could be swept into the agent's commit.
  • Slug guard accepts ~-prefixed "latest" aliases (the org value is ~deepseek/deepseek-flash-latest; the first draft rejected them and would have failed every act run) and :free-style variants.

2. Auto-approval was never wired (1427f3d)

Every review run ended with [debug1] All reviewers agree the patch is correct; no approval token configured: the workflow passed no github_approval_token, and no DOTBOT_GITHUB_USER_PAT exists (0 repo secrets; the org secret does not exist either). REPO_ACCESS_TOKEN belongs to the PR author, whose approvals GitHub rejects — why the machine-user wiring was dropped in a26dc15. Passing github.token is what the repo permits (can_approve_pull_request_reviews: true).

3. The pin silently dropped the input (3665e57, 863820f)

wezell/openrouter-code-review-action@d68edbb predates the github_approval_token input, so GitHub discarded it: ##[warning]Unexpected input(s) 'github_approval_token'. wezell/… is no longer the release source (its v1/latest = 34ee169, v1.0.0-era). Both workflows now pin the canonical dotCMS/openrouter-code-review-action@bbe2345 (v1.2.0 / v1 / latest), and a test asserts both workflows pin the same released SHA. Supersedes #4.

4. Installation tokens could not submit the approval (b6ed738)

Warning: failed to submit PR approval: failed to resolve approval token user:
Resource not accessible by integration: 403

submit_pr_approval resolved the token's login via GET /user first; installation tokens cannot call it. Login resolution is now best-effort and idempotency is keyed off APPROVAL_MARKER in the review body as well, so re-runs still skip duplicates when the approver identity is unreadable.

5. P0: the checkout shadowed the pinned action (5bd98e4)

Composite run steps execute in the caller's workspace and python -m puts the cwd ahead of PYTHONPATH, so python3 -u -m cli.main loaded cli/ from the checkout whenever the consuming repo ships one. Evidence from this PR: the run at b6ed738 printed Approved PR #7: all reviewers agree the patch is correct — the new message format from that very commit — and submitted an approval the pinned revision cannot submit.

For dotbot-act.yml that is the exact escalation the trusted-ref pin exists to prevent: a /dotbot comment on an untrusted PR head would run that PR's cli/ package with contents: write and REPO_ACCESS_TOKEN. Fix: PYTHONSAFEPATH=1 on both CLI-invoking steps, plus a test that every CLI step in action.yml carries it.

$ cd work && PYTHONPATH=…/action python3 -m cli.main                    # ran from: work
$ cd work && PYTHONSAFEPATH=1 PYTHONPATH=…/action python3 -m cli.main  # ran from: action

Verification

  • uv run pytest -q → 806 passed; pre-commit run --all-files (ruff format/check, mypy) → Passed
  • tests/test_dotbot_workflow_model_vars.py executes the act step's real run: script under bash and parses the generated config back through load_model_config
  • Review runs on this PR: unanimous "patch is correct", 0 findings, and github-actions APPROVED at the head SHA

Merge status

Merge-ready: all checks pass, reviewDecision: APPROVED, mergeStateStatus: CLEAN, and every review thread resolved.

Correction to an earlier note in this PR: the ruleset block was not the approval identity — github-actions[bot]'s approval does count (reviewDecision flipped to APPROVED with it). The PR was blocked by required_review_thread_resolution: the two unaddressed dotbot findings were still open. Once answered and resolved, the PR went straight to CLEAN. Worth remembering that dotbot threads have to be resolved, not just fixed.

After merge, auto-release.yml cuts v1.3.0 and retags v1/latest; a follow-up bumps the self-review pins to that SHA so the PYTHONSAFEPATH and approval fixes apply to the self-review itself (until then this PR's own runs still execute the checkout's CLI — see commit 5).

Review already takes its model from the DOTBOT_REVIEW_MODELS variable (first
entry = primary reviewer, the rest = the "fight" roster), rendered into
.openrouter-review.yml for the run. Act had no equivalent: the act model could
only be changed by editing act.model in the consuming repo's committed config.

Add the act counterpart:

* dotbot-act.yml renders `act:\n  model: <slug>` from vars.DOTBOT_ACT_MODEL and
  passes it through the `config_path` action input. The variable is a
  REPLACEMENT, not an addition - when set it is authoritative, when unset the
  in-repo file (or action default) stands.
* The generated file is written to $RUNNER_TEMP, NOT the checkout: act mode
  pushes commits to the PR branch, so a generated file in the worktree could be
  swept into the agent's commit and overwrite the repo's own config. Review can
  render in place because review never commits.
* Slugs are validated (`[A-Za-z0-9._:/-]+`) so a malformed variable fails the
  run instead of producing a broken config file.

Tests: workflow wiring assertions for both variables, plus model-config tests
proving an act-only generated file outside GITHUB_WORKSPACE is loaded and that
it wins over the repo's committed act.model.

Docs: new "Org/Repo Variables (vars.DOTBOT_*)" README section covering both
variables, their precedence, and why act renders outside the checkout.
The dotCMS org variable carries ~deepseek/deepseek-flash-latest, and the `~`
"latest" alias prefix is a normal part of OpenRouter slugs — the first revision
of the guard rejected it and would have failed every act run.

* Widen the slug guard to [A-Za-z0-9._:/~@+-].
* Emit the rendered value as a single-quoted YAML scalar, so unusual-but-legal
  slugs survive the round trip.
* Add tests/test_dotbot_workflow_model_vars.py: executes the workflow step's real
  `run:` script under bash and asserts the generated config (parsed back through
  load_model_config) for ~latest aliases, :free variants, whitespace stripping,
  the blank-variable fallback, and the non-slug failure path. Assertions on the
  script's internals in test_module_coverage.py are dropped in favour of this.
@wezell

wezell commented Sep 29, 2026

Copy link
Copy Markdown
Member Author

Follow-up commit 4fadf9f: the org variable's actual value is ~deepseek/deepseek-flash-latest, and OpenRouter's ~-prefixed "latest" aliases are ordinary slugs — the first revision of the guard would have rejected it and failed every act run.

  • Slug guard widened to [A-Za-z0-9._:/~@+-]; the rendered value is now a single-quoted YAML scalar.
  • Added tests/test_dotbot_workflow_model_vars.py, which executes the workflow step's real run: script under bash and parses the generated config back through load_model_config — covering ~deepseek/deepseek-flash-latest, ~z-ai/glm-latest, :free variants, whitespace stripping, the blank-variable fallback (warn + no config_path), and the non-slug failure path.

Verified against the current org values:

  • DOTBOT_ACT_MODEL=~deepseek/deepseek-flash-latest → act runs resolve to that slug
  • DOTBOT_REVIEW_MODELS=meta/muse-spark-1.3,~z-ai/glm-latest → unchanged (already wired)

uv run pytest -q → 802 passed; pre-commit run --all-files (ruff format/check, mypy) → Passed.

The self-review workflow passed no `github_approval_token`, so every run ended
with "[debug1] All reviewers agree the patch is correct; no approval token
configured" and dotbot never approved: PRs needing 1 approving review stalled.

This repo has no DOTBOT_GITHUB_USER_PAT (0 repo secrets; the org secret does not
exist either), and REPO_ACCESS_TOKEN belongs to the PR author, whose approvals
GitHub rejects — which is why the earlier machine-user wiring was dropped in
a26dc15. The repo does allow Actions to approve PRs
(can_approve_pull_request_reviews: true), so pass `github.token` instead: the
approval lands as github-actions[bot] and satisfies the ruleset's 1-approval /
last-push-approval requirements.

Tradeoff documented in the workflow comment and README: a bot identity cannot
approve a PR the bot authored, and a machine-user PAT remains the better option
where one exists.
Comment thread .github/workflows/dotbot-review.yml
Comment thread .github/workflows/dotbot-review.yml
Pinning to d68edbb (the split-review-act era) meant GitHub silently rejected
inputs that revision did not declare:

  ##[warning]Unexpected input(s) 'github_approval_token', valid inputs are [...]

So `github_approval_token: ${{ github.token }}` never reached the action and
auto-approval still did not fire after the previous commit. Bump both workflows
to bbe2345 (v1.2.0 / v1 / latest), which declares the inputs the workflows pass.

Also assert in tests that both workflows pin the same released commit — drift
between them is how the review side ended up on a pre-approval revision.
wezell/openrouter-code-review-action is no longer the source of releases: its
main/v1/latest sit at 34ee169 (v1.0.0-era, no github_approval_token input), and
it has no release carrying that input. So after the previous two commits the run
died earlier still:

  ##[error]Unable to resolve action
  `wezell/openrouter-code-review-action@bbe2345...`

This repo is canonical and cuts its own releases (auto-release.yml), so point
both workflows at bbe2345... (v1.2.0 / v1 /
latest), which declares every input the workflows pass, and require the canonical
owner in tests. Supersedes the still-open PR #4, which pinned the review workflow
to v1.1.0 and left act on wezell.
With the input now wired, the run got as far as:

  Warning: failed to submit PR approval: failed to resolve approval token user:
  Resource not accessible by integration: 403

`submit_pr_approval` resolved the token's login via `GET /user` before doing
anything else. Installation tokens (`${{ github.token }}`) cannot call that
endpoint, so the approval was abandoned — the input reached the action and was
still useless.

- `_resolve_login` is now best-effort: an unresolvable token logs a debug line
  and continues with an empty login instead of raising.
- Idempotency is keyed off APPROVAL_MARKER ("approved automatically by dotbot")
  in the review body in addition to the login, so re-runs skip duplicate
  approvals even when the approver identity cannot be read. Extracted the
  APPROVAL_MARKER constant so the body template and the check cannot drift.
- Failure to load the PR still raises (unchanged); the approval POST failure
  message names the token as an installation token when the login is unknown,
  and the workflow's log lines omit "as <login>" rather than printing an empty
  name.

Also documents the choice in action.yml, README, and the `_maybe_approve_pr`
docstring.
github-actions[bot]
github-actions Bot previously approved these changes Sep 29, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

approved automatically by dotbot

Composite `run` steps execute in the caller's workspace, and `python -m` puts
the current directory ahead of PYTHONPATH on sys.path. So `python3 -u -m cli.main`
loaded `cli/` from the *checkout* whenever the consuming repo shipped one — for
this repo's own self-review, the PR head's code ran with the workflow token
instead of the pinned action. Demonstrated on PR #7: the run reported
"Approved PR #7: all reviewers agree the patch is correct" (new message format
from that very PR), and it submitted an approval the pinned revision cannot
submit (its approval.py aborts on GET /user with 403).

For the act workflow this is the escalation the trusted-ref pin exists to
prevent: a /dotbot comment on an untrusted PR head would execute that PR's
`cli/` package with `contents: write` and REPO_ACCESS_TOKEN.

Set PYTHONSAFEPATH=1 on both CLI-invoking steps (3.11+: do not prepend cwd to
sys.path), and add a test that every CLI step in action.yml carries it.

Repro:

  mkdir -p action/cli work/cli; ...   # both define cli.main
  cd work && PYTHONPATH=…/action python3 -m cli.main      # -> ran from: work
  cd work && PYTHONSAFEPATH=1 PYTHONPATH=…/action python3 -m cli.main  # -> action
@github-actions

Copy link
Copy Markdown

dotbot code review:

  • Reviewer: meta/muse-spark-1.3 (medium)
  • Overall: patch is correct
  • New findings this run: 0
  • Prior unresolved dotbot findings still relevant: 0
  • Active findings total: 0

PYTHONSAFEPATH on both CLI-invoking composite steps correctly prevents checkout cli/ shadowing while preserving PYTHONPATH lookup; coverage test enforces the invariant.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

reviewed by dotbot · meta/muse-spark-1.3 · medium

@github-actions

Copy link
Copy Markdown

dotbot code review:

  • Reviewer: ~z-ai/glm-latest (medium)
  • Overall: patch is correct
  • New findings this run: 0
  • Prior unresolved dotbot findings still relevant: 0
  • Active findings total: 0

PYTHONSAFEPATH=1 is set on both CLI-invoking steps (prepare_resume_state and cli.main), correctly preventing python -m from prepending the caller's cwd to sys.path, so the pinned action's cli/ package (first on PYTHONPATH via GITHUB_ACTION_PATH) wins over a checkout that ships its own. The new coverage test pins this invariant for both steps, and no other step executes repo-reachable Python modules.

Tip: comment with "/dotbot address comments" to attempt automated fixes for unresolved review threads.

reviewed by dotbot · ~z-ai/glm-latest · medium

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ dotbot review: all reviewer models (meta/muse-spark-1.3, ~z-ai/glm-latest) agree — patch is correct.

approved automatically by dotbot

@wezell wezell changed the title feat: drive act model from DOTBOT_ACT_MODEL org/repo variable feat: DOTBOT_ACT_MODEL for the act model, plus the fixes that unblock dotbot approval Sep 29, 2026
@wezell
wezell merged commit b1c5b6a into main Sep 29, 2026
4 checks passed
wezell added a commit that referenced this pull request Sep 29, 2026
v1.3.1 (3f4cfa1) is the first release whose action.yml loads: v1.3.0 shipped a
`${{ github.token }}` in an input description, which made GitHub reject the whole
action, so pinning 1.3.0 left this repo's own review/act jobs failing at
"Set up job" (that is what this PR's earlier run hit).

v1.3.1 carries everything from #7 plus the description fix, so the self-review now
runs the released action instead of executing the checkout's cli/ package.
v1/latest point at the same commit; the test constant moves with the pins.
wezell added a commit that referenced this pull request Sep 29, 2026
v1.3.1 (3f4cfa1) is the first release whose action.yml loads: v1.3.0 shipped a
`${{ github.token }}` in an input description, which made GitHub reject the whole
action, so pinning v1.3.0 left this repo's own review/act jobs failing at
"Set up job" (that is what this PR's earlier run hit).

v1.3.1 carries everything from #7 plus the description fix, so the self-review now
runs the released action instead of executing the checkout's cli/ package.
v1/latest point at the same commit; the test constant moves with the pins.
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.

1 participant