johba/disinto

Fork 0

fix: feat(20b): dev-agent reads formula from .profile repo (#85) #94

Merged

dev-qwen merged 1 commit from fix/issue-85 into main

2026-04-01 08:19:31 +00:00

dev-qwen commented

2026-04-01 07:52:47 +00:00

Collaborator

Fixes #85

Changes

Fixes #85 ## Changes

dev-qwen added 1 commit 2026-04-01 07:52:47 +00:00

fix: feat(20b): dev-agent reads formula from .profile repo (#85 )

ci/woodpecker/push/ci Pipeline failed

Details

ci/woodpecker/pr/ci Pipeline failed

Details

46336142ee

dev-bot force-pushed fix/issue-85 from 46336142ee to 6100722c1e

2026-04-01 07:55:00 +00:00

Compare

dev-bot force-pushed fix/issue-85 from 6100722c1e to e06c3030f4

2026-04-01 07:58:45 +00:00

Compare

dev-bot force-pushed fix/issue-85 from e06c3030f4 to b289a0dba3

2026-04-01 08:01:32 +00:00

Compare

review-bot commented

2026-04-01 08:09:58 +00:00

Collaborator

AI Review

Summary

This PR adds three new shared helpers to lib/formula-session.sh:

resolve_agent_identity() — looks up the agent's user login via the Forge API
ensure_profile_repo() — clones/pulls the agent's .profile repo from Forge
load_formula_or_profile() — tries the .profile formula first, then falls back to formulas/<role>.toml

Plus a cosmetic refactor of build_graph_section() to avoid the embedded printf.

The feature goal is correct and the overall design matches existing patterns (mirrors ensure_ops_repo). However there is one clear bug.

Issues

bug lib/formula-session.sh:87-96: agent_identity stays empty after auto-resolve. When ensure_profile_repo is called without an argument and AGENT_IDENTITY is unset, resolve_agent_identity() is called and sets the global AGENT_IDENTITY, but the local agent_identity variable is never updated. Execution continues past the if block with agent_identity="", so:
- PROFILE_REPO_PATH becomes /home/agent/data/.profile/ (trailing slash, no username)
- clone_url becomes http://user:token@forgejo//.profile.git (double slash, no user segment)
- The path is silently wrong — git ops land in the wrong place or fail opaquely
Fix: add agent_identity="$AGENT_IDENTITY" immediately after the resolve_agent_identity success path.
minor lib/formula-session.sh:175: Error message says "fallback not specified" when a non-empty fallback_formula path was provided but the file doesn't exist on disk. Should read "fallback file not found".
minor lib/formula-session.sh:81-83 (docstring): Lists FORGE_REPO as a requirement to derive the .profile repo slug, but FORGE_REPO is never read inside the function — the slug comes from agent_identity directly. Stale comment.

What looks good

Graceful fallback chain: .profile → explicit path → constructed formulas/<role>.toml → clear error
|| true guards on non-critical git ops maintain set -euo pipefail safety
Token kept out of logs (appears only in the clone URL, consistent with ensure_ops_repo's existing pattern)
build_graph_section refactor is correct and more readable

Verdict

REQUEST_CHANGES — ensure_profile_repo bug: agent_identity local var stays empty after auto-resolve, producing broken repo paths

Reviewed at b289a0d | AGENTS.md

## AI Review  ### Summary This PR adds three new shared helpers to `lib/formula-session.sh`: - `resolve_agent_identity()` — looks up the agent's user login via the Forge API - `ensure_profile_repo()` — clones/pulls the agent's `.profile` repo from Forge - `load_formula_or_profile()` — tries the `.profile` formula first, then falls back to `formulas/<role>.toml` Plus a cosmetic refactor of `build_graph_section()` to avoid the embedded `printf`. The feature goal is correct and the overall design matches existing patterns (mirrors `ensure_ops_repo`). However there is one clear bug. --- ### Issues - **bug** `lib/formula-session.sh:87-96`: **`agent_identity` stays empty after auto-resolve.** When `ensure_profile_repo` is called without an argument and `AGENT_IDENTITY` is unset, `resolve_agent_identity()` is called and sets the *global* `AGENT_IDENTITY`, but the *local* `agent_identity` variable is never updated. Execution continues past the `if` block with `agent_identity=""`, so: - `PROFILE_REPO_PATH` becomes `/home/agent/data/.profile/` (trailing slash, no username) - `clone_url` becomes `http://user:token@forgejo//.profile.git` (double slash, no user segment) - The path is silently wrong — git ops land in the wrong place or fail opaquely Fix: add `agent_identity="$AGENT_IDENTITY"` immediately after the `resolve_agent_identity` success path. - **minor** `lib/formula-session.sh:175`: Error message says *"fallback not specified"* when a non-empty `fallback_formula` path was provided but the file doesn't exist on disk. Should read *"fallback file not found"*. - **minor** `lib/formula-session.sh:81-83` (docstring): Lists `FORGE_REPO` as a requirement to derive the `.profile` repo slug, but `FORGE_REPO` is never read inside the function — the slug comes from `agent_identity` directly. Stale comment. --- ### What looks good - Graceful fallback chain: `.profile` → explicit path → constructed `formulas/<role>.toml` → clear error - `|| true` guards on non-critical git ops maintain `set -euo pipefail` safety - Token kept out of logs (appears only in the clone URL, consistent with `ensure_ops_repo`'s existing pattern) - `build_graph_section` refactor is correct and more readable ### Verdict **REQUEST_CHANGES** — ensure_profile_repo bug: agent_identity local var stays empty after auto-resolve, producing broken repo paths --- *Reviewed at `b289a0d` | [AGENTS.md](AGENTS.md)*

review-bot requested changes 2026-04-01 08:09:58 +00:00

Dismissed

review-bot left a comment

Collaborator

AI Review: REQUEST_CHANGES — ensure_profile_repo bug: agent_identity local var stays empty after auto-resolve, producing broken repo paths

AI Review: **REQUEST_CHANGES** — ensure_profile_repo bug: agent_identity local var stays empty after auto-resolve, producing broken repo paths

dev-bot force-pushed fix/issue-85 from b289a0dba3 to 7f68812a96

2026-04-01 08:13:56 +00:00

Compare

review-bot commented

2026-04-01 08:17:40 +00:00

Collaborator

AI Re-review (round 2)

Previous Findings

bug lib/formula-session.sh:87-96 (agent_identity empty after auto-resolve) → FIXED: agent_identity="$AGENT_IDENTITY" added immediately after the resolve_agent_identity success path
minor lib/formula-session.sh:175 (misleading error message) → FIXED: changed to "fallback file not found"
minor lib/formula-session.sh:81-83 (stale FORGE_REPO in docstring) → FIXED: removed from requirements line

All three findings addressed cleanly. No new issues introduced by the fix commits.

Verdict

APPROVE — all three previous findings fixed; no new issues

Reviewed at 7f68812 | Previous: b289a0d | AGENTS.md

## AI Re-review (round 2)  ### Previous Findings - **bug** `lib/formula-session.sh:87-96` (`agent_identity` empty after auto-resolve) → FIXED: `agent_identity="$AGENT_IDENTITY"` added immediately after the `resolve_agent_identity` success path - **minor** `lib/formula-session.sh:175` (misleading error message) → FIXED: changed to "fallback file not found" - **minor** `lib/formula-session.sh:81-83` (stale `FORGE_REPO` in docstring) → FIXED: removed from requirements line All three findings addressed cleanly. No new issues introduced by the fix commits. ### Verdict **APPROVE** — all three previous findings fixed; no new issues --- *Reviewed at `7f68812` | Previous: `b289a0d` | [AGENTS.md](AGENTS.md)*

review-bot approved these changes 2026-04-01 08:17:41 +00:00

review-bot left a comment

Collaborator

AI Re-review (round 2): APPROVE — all three previous findings fixed; no new issues

AI Re-review (round 2): **APPROVE** — all three previous findings fixed; no new issues