add xtrace.bsh, benchmark.bsh, trace.bsh shared helpers#16
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f71e4319cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| local repo | ||
| repo="$(basename -- "$repo_dir")" | ||
|
|
||
| if ! ghorg_repo_exists "$target_owner" "$repo"; then |
There was a problem hiding this comment.
Treat non-404 repo lookup failures as fatal in push
push_one treats any falsey result from ghorg_repo_exists as "repo missing", but ghorg_repo_exists returns false for all non-200 statuses (including 401/403/rate-limit/API failures), not just 404. In those cases the script skips pushes and continues with a misleading "does not exist" warning, so a broken token or transient API error can silently skip an entire run.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fa101e1. Replaced ghorg_repo_exists with ghorg_repo_lookup returning three distinct codes (0=exists, 1=404, 2=other failure). push_one now treats code 2 as a hard per-repo error and refuses to push, so a bad token or transient API failure surfaces loudly instead of being silently mistaken for "missing".
Generated by Claude Code
| body="$(jq -n --arg org "$target_owner" \ | ||
| '{organization: $org, default_branch_only: false}')" | ||
| else | ||
| body="$(jq -n '{default_branch_only: false}')" |
There was a problem hiding this comment.
Reject mismatched target user when creating forks
When target_owner is a User, the code omits the organization field and calls POST /repos/{owner}/{repo}/forks; GitHub creates that fork for the authenticated user, not an arbitrary username. Because there is no check that target_owner matches the token user, passing a different user name can create forks in the wrong account while logs still claim they were created under target_owner.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fa101e1. Added ghorg_authenticated_user (calls GET /user, caches the login). github-org-fork's main() now aborts up front when the target is a User but doesn't equal the auth user, so the run never reaches fork_one with a mismatched target. fork_one keeps a defensive check too.
Generated by Claude Code
| account_type="$(ghorg_account_type "$owner")" || return 1 | ||
| case "$account_type" in | ||
| Organization) api_base="/orgs/${owner}/repos?per_page=100&type=all" ;; | ||
| User) api_base="/users/${owner}/repos?per_page=100" ;; |
There was a problem hiding this comment.
Use the right endpoint for private repos on user owners
The User branch in ghorg_list_repos always uses /users/{username}/repos, which only lists public repositories. This makes --include-private ineffective for user-owned sources and also breaks fork idempotence for user targets with private repos, because existing private forks are never seen in target_repos and can be retried as if missing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in fa101e1. When the listed owner is a User and --include-private is set, ghorg_list_repos now switches to /user/repos?affiliation=owner (the only endpoint that returns private repos). Since that endpoint only ever returns the authenticated user's repos, the function refuses with a clear error when the listed user differs from the auth user — surfacing the limitation instead of silently returning a public-only subset.
Generated by Claude Code
P1 (push): Distinguish missing from lookup-failed. ghorg_repo_exists
collapsed every non-200 status into "missing", so a bad token or
rate-limit silently no-op'd entire pushes. Replaced with
ghorg_repo_lookup returning 0=exists, 1=404, 2=other-error.
push_one treats code 2 as a hard per-repo error rather than skipping.
P1 (fork): Refuse mismatched User target. When the target owner is
a User account, GitHub forks to the *authenticated* user regardless
of the target_owner argument. Added ghorg_authenticated_user; main()
now aborts up front if target is a User but doesn't equal the auth
user. fork_one carries a defensive check too.
P2 (lib): /users/{u}/repos returns only public repos, so
--include-private was silently ineffective for User owners and
existing private forks were re-detected as missing. ghorg_list_repos
now switches to /user/repos?affiliation=owner when listing private
repos owned by the authenticated user (the only case the API
supports), and refuses with a clear error when the listed user
differs from the auth user.
dc522dc to
0fe8fa6
Compare
Three small generic shell helper libraries used by derivative-maker/help-steps/pre, which currently inlines them. Migrating them here lets other tools reuse them and shrinks pre significantly. All three are sourceable via: source /usr/libexec/helper-scripts/<lib>.bsh xtrace.bsh: xtrace_off / xtrace_restore / output_cmd_set Suspend/restore set -x around blocks that must not be echoed (secret handling, noisy output). Saved-state global is helper_xtrace_prev_state (project-neutral name; renamed from derivative-maker's dist_build_xtrace_prev_state). benchmark.bsh: benchmark_format / benchmark_since Format integer seconds as 0s / 42s / 3m42s / 1h03m42s, and a convenience that takes a $SECONDS snapshot and reports the elapsed. trace.bsh: function_trace / process_backtrace Diagnostic backtrace helpers for error handlers. Renamed from function_trace_function / process_backtrace_function and switched from writing to globals (function_trace_result / process_backtrace_result) to printing the result on stdout, so callers capture into a local: trace=\"\$(function_trace)\". Both diagnostic helpers belong together; one source line gets both. Smoke-tested locally: function_trace correctly walks the caller's stack even though the definition lives in a sourced lib (caller reports the invoker's frames, not the lib's). benchmark_format handles 0, 42, 245, 3725 -> 0s, 42s, 4m05s, 1h02m05s.
27aa9d4 to
83e439b
Compare
Summary
Three small generic shell helper libraries used by
derivative-maker/help-steps/pre(which currently inlines them). Migrating them here lets other tools reuse them and shrinkspresignificantly. The github-org-* tools that originally lived on this branch have been moved out todeveloper-meta-files(PR assisted-by-ai/developer-meta-files#3); this PR is now lib-only.Files added
usr/libexec/helper-scripts/xtrace.bsh--xtrace_off/xtrace_restore/output_cmd_set. Suspend/restoreset -xaround blocks that must not be echoed (secret handling, noisy output). Saved-state global ishelper_xtrace_prev_state(project-neutral name).usr/libexec/helper-scripts/benchmark.bsh--benchmark_format/benchmark_since. Format integer seconds as0s/42s/3m42s/1h03m42s; convenience that takes a$SECONDSsnapshot and reports elapsed.usr/libexec/helper-scripts/trace.bsh--function_trace/process_backtrace. Diagnostic backtrace helpers for error handlers. Renamed fromfunction_trace_function/process_backtrace_functionand switched from writing to globals to printing the result on stdout, so callers capture into a local:trace="$(function_trace)".Files removed
The github-org-* tools and their shared library (
github-org-{clone,fork,push,set-fork-approval},github-org-lib.bsh) moved todeveloper-meta-files(PR assisted-by-ai/developer-meta-files#3). They are co-located there with thedm-*wrappers that consume them.sanitize-stringand its supporting Python packages (stdisplay,strip_markup) remain here -- they are general-purpose string-safety utilities used elsewhere inhelper-scriptsand now also a runtime dependency of the moved github-org-lib's safe-print helper.Companion PRs
help-steps/preto source the new libs and removes the inline copies. Submodule pointer bump deferred to maintainer (see that PR for details).Test plan
bash -npasses on each new lib (verified locally).xtrace_off/xtrace_restore(round-trip preserves state).benchmark_formatfor 0s / 42s / 245s / 3725s -> 0s / 42s / 4m05s / 1h02m05s.function_tracefrom a nested function call -- correctly walks the caller's stack.process_backtrace-- prints the /proc-walk back to PID 1.https://claude.ai/code