Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 31 additions & 28 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -322,16 +322,24 @@ define _dispatch
case "$$what" in \
*[!a-z0-9_-]*|'') printf 'ERROR: invalid WHAT selector %s\n' "$$what" >&2; exit 2 ;; \
esac; \
case " $(_ALLOWED_WHATS_$(1)) " in \
*" $$what "*) ;; \
*) printf 'ERROR: unsupported %s WHAT=%s (allowed:%s)\n' "$(1)" "$$what" "$(_ALLOWED_WHATS_$(1))" >&2; exit 2 ;; \
esac; \
custom="_custom_$(1)_$$what"; \
$(SELF_MAKE) -q "$$custom" >/dev/null 2>&1; custom_rc=$$?; \
if [ "$$custom_rc" -eq 2 ]; then \
case " $(_ALLOWED_WHATS_$(1)) " in \
*" $$what "*) ;; \
*) printf 'ERROR: unsupported %s WHAT=%s (allowed:%s)\n' "$(1)" "$$what" "$(_ALLOWED_WHATS_$(1))" >&2; exit 2 ;; \
esac; \
fi; \
builtin="_builtin_$(1)_$$what"; \
for hook in "pre-$(1)" "pre-$(1)-$$what"; do \
$(SELF_MAKE) -q "$$hook" >/dev/null 2>&1; rc=$$?; \
if [ "$$rc" -ne 2 ]; then $(SELF_MAKE) "$$hook" || exit $$?; fi; \
done; \
$(SELF_MAKE) "$$builtin" || exit $$?; \
if [ "$$custom_rc" -ne 2 ]; then \
$(SELF_MAKE) "$$custom" || exit $$?; \
else \
$(SELF_MAKE) "$$builtin" || exit $$?; \
fi; \
for hook in "post-$(1)-$$what" "post-$(1)"; do \
$(SELF_MAKE) -q "$$hook" >/dev/null 2>&1; rc=$$?; \
if [ "$$rc" -ne 2 ]; then $(SELF_MAKE) "$$hook" || exit $$?; fi; \
Expand Down Expand Up @@ -505,8 +513,9 @@ _builtin_help_usage:
@printf '\n%s\n' 'Custom hooks (custom.mk):';
@printf ' %s\n' 'Define pre-<verb>, post-<verb>, pre-<verb>-<what>, post-<verb>-<what>';
@printf ' %s\n' 'in custom.mk to wrap one declared handler.';
@printf ' %s\n' 'Add _custom_<verb>_<what> to define a new WHAT.';
@if [ -f custom.mk ]; then \
hooks=$$(grep -oE '^(pre|post)-[a-z][a-z0-9-]*' custom.mk 2>/dev/null | sort -u); \
hooks=$$(grep -oE '^(pre|post)-[a-z][a-z0-9-]*|^_custom_[a-z][a-z0-9_-]*' custom.mk 2>/dev/null | sort -u); \
if [ -n "$$hooks" ]; then \
printf ' %s\n' 'Defined in this project:'; \
for hook in $$hooks; do printf ' %s\n' "$$hook"; done; \
Expand All @@ -527,8 +536,10 @@ _builtin_help_usage:
# An absent checkout holds no work, so setup initializes it at the recorded
# gitlink. A present checkout is never destroyed: git checkout and git reset
# are forbidden. Detached HEAD is attached via branch + symbolic-ref so dirty
# work is carried. branch = . follows the superproject named branch (worktree
# propagation). Fetch is conditional when cached origin refs already validate.
# work is carried. Pin validity is HEAD contains gitlink — origin may lag the
# pin without failing verify. Declared branch is the named integration line;
# legacy branch=. still resolves to the superproject named branch if present.
# Fetch skips when local already contains pin and origin tip.
# Free: no
# End SECTION: submodule setup
_builtin_setup_submodules:
Expand Down Expand Up @@ -623,18 +634,10 @@ _builtin_setup_submodules:
exit 1; \
fi; \
need_fetch=1; \
if git -C "$$child_root" rev-parse --verify "$$remote_ref" >/dev/null 2>&1; then \
cached_ok=1; \
if [ -z "$$current" ]; then \
if ! git -C "$$child_root" merge-base --is-ancestor "$$head" "$$remote_ref"; then \
cached_ok=0; \
fi; \
fi; \
if [ "$$cached_ok" -eq 1 ] && \
git -C "$$child_root" merge-base --is-ancestor "$$gitlink" "$$remote_ref" && \
git -C "$$child_root" merge-base --is-ancestor "$$gitlink" HEAD; then \
need_fetch=0; \
fi; \
if git -C "$$child_root" rev-parse --verify "$$remote_ref" >/dev/null 2>&1 && \
git -C "$$child_root" merge-base --is-ancestor "$$gitlink" HEAD && \
git -C "$$child_root" merge-base --is-ancestor "$$remote_ref" HEAD; then \
need_fetch=0; \
fi; \
if [ "$$need_fetch" -eq 1 ]; then \
git -C "$$child_root" fetch --quiet origin "$$branch" || { \
Expand All @@ -649,19 +652,19 @@ _builtin_setup_submodules:
exit 1; \
fi; \
if [ -z "$$current" ]; then \
if ! git -C "$$child_root" merge-base --is-ancestor "$$head" "$$remote_ref"; then \
printf 'ERROR: %s: detached HEAD %s is not contained in origin/%s; reconcile it yourself (setup never discards commits)\n' "$$child_path" "$$head" "$$branch" >&2; \
if git -C "$$child_root" merge-base --is-ancestor "$$gitlink" HEAD; then \
attach_branch_at_head "$$child_root" "$$branch"; \
elif git -C "$$child_root" rev-parse --verify "$$remote_ref" >/dev/null 2>&1 && \
git -C "$$child_root" merge-base --is-ancestor "$$head" "$$remote_ref"; then \
attach_branch_at_head "$$child_root" "$$branch"; \
Comment on lines +655 to +659

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not force-move an existing declared branch.

attach_branch_at_head runs git branch -f "$$branch" HEAD. If refs/heads/$$branch already points to local commits, either detached-HEAD path can replace that ref with $$head. The prior commits become unreachable and can later be garbage-collected. This conflicts with the stated rule that setup does not discard commits.

Reject this state when the declared local branch exists at a different commit. Require the user to reconcile it before attaching HEAD.

Proposed safeguard
 attach_branch_at_head() { \
   child_root="$$1"; \
   branch="$$2"; \
+  head=$$(git -C "$$child_root" rev-parse HEAD); \
+  existing=$$(git -C "$$child_root" rev-parse --verify "refs/heads/$$branch" 2>/dev/null || :); \
+  if [ -n "$$existing" ] && [ "$$existing" != "$$head" ]; then \
+    printf 'ERROR: %s: local branch %s points to %s, not detached HEAD %s; reconcile it yourself\n' "$$child_root" "$$branch" "$$existing" "$$head" >&2; \
+    exit 1; \
+  fi; \
   git -C "$$child_root" branch --quiet -f "$$branch" HEAD || { \
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 655 - 659, Guard both attach_branch_at_head call sites
in the detached-HEAD handling flow so an existing declared branch is accepted
only when it already points to the intended commit; if refs/heads/$$branch
exists at a different commit, reject the state and require manual reconciliation
before attaching HEAD. Preserve attachment when the branch is absent or already
matches, and ensure attach_branch_at_head is never allowed to overwrite
divergent local commits.

else \
printf 'ERROR: %s: detached HEAD %s is not on the recorded gitlink and not contained in origin/%s; reconcile it yourself (setup never discards commits)\n' "$$child_path" "$$head" "$$branch" >&2; \
exit 1; \
fi; \
attach_branch_at_head "$$child_root" "$$branch"; \
current="$$branch"; \
fi; \
if ! git -C "$$child_root" merge-base --is-ancestor "$$gitlink" "$$remote_ref"; then \
printf 'ERROR: %s: origin/%s diverges from recorded gitlink %s\\n' "$$child_path" "$$branch" "$$gitlink" >&2; \
exit 1; \
fi; \
if ! git -C "$$child_root" merge-base --is-ancestor "$$gitlink" HEAD; then \
printf 'ERROR: %s: branch %s diverges from recorded gitlink %s\n' "$$child_path" "$$branch" "$$gitlink" >&2; \
printf 'ERROR: %s: branch %s diverges from recorded gitlink %s (setup never runs checkout/reset; advance or switch it yourself while keeping dirty)\n' "$$child_path" "$$branch" "$$gitlink" >&2; \
exit 1; \
fi; \
if [ -f "$$child_root/.gitmodules" ]; then \
Expand Down
27 changes: 13 additions & 14 deletions src/flext_cli/_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,10 @@

from typing import Annotated, ClassVar

from pydantic import Field
from pydantic_settings import SettingsConfigDict

from flext_cli._constants.settings import FlextCliConstantsSettings
from flext_core import FlextSettings
from flext_core import FlextSettings, m


class FlextCliSettings(FlextSettings):
Expand All @@ -28,42 +27,42 @@ class FlextCliSettings(FlextSettings):
env_prefix="FLEXT_CLI_", extra="ignore"
)

cli_verbose: Annotated[bool, Field(description="Verbose output")] = (
cli_verbose: Annotated[bool, m.Field(description="Verbose output")] = (
FlextCliConstantsSettings.CLI_DEFAULT_VERBOSE
)
cli_quiet: Annotated[bool, Field(description="Quiet output")] = (
cli_quiet: Annotated[bool, m.Field(description="Quiet output")] = (
FlextCliConstantsSettings.CLI_DEFAULT_QUIET
)
cli_app_name: Annotated[str, Field(description="CLI application name")] = (
cli_app_name: Annotated[str, m.Field(description="CLI application name")] = (
FlextCliConstantsSettings.FLEXT_CLI
)
cli_log_verbosity: Annotated[
str, Field(description="Log format (compact, detailed, full)")
str, m.Field(description="Log format (compact, detailed, full)")
] = FlextCliConstantsSettings.CLI_DEFAULT_LOG_VERBOSITY
cli_log_level: Annotated[str, Field(description="CLI log level")] = (
cli_log_level: Annotated[str, m.Field(description="CLI log level")] = (
FlextCliConstantsSettings.CLI_DEFAULT_LOG_LEVEL
)
cli_no_color: Annotated[bool, Field(description="Disable colored output")] = (
cli_no_color: Annotated[bool, m.Field(description="Disable colored output")] = (
FlextCliConstantsSettings.CLI_DEFAULT_NO_COLOR
)
cli_output_format: Annotated[
str, Field(description="Output format (table, json, yaml, csv, plain)")
str, m.Field(description="Output format (table, json, yaml, csv, plain)")
] = FlextCliConstantsSettings.CLI_DEFAULT_OUTPUT_FORMAT
cli_config_file: Annotated[
str | None, Field(description="Path to settings file")
str | None, m.Field(description="Path to settings file")
] = None
cli_token_file: Annotated[
str | None, Field(description="Path to auth token file")
str | None, m.Field(description="Path to auth token file")
] = None
cli_ci: Annotated[
bool, Field(description="Whether the current runtime is a CI environment.")
bool, m.Field(description="Whether the current runtime is a CI environment.")
] = False
cli_pytest_current_test: Annotated[
str | None, Field(description="Current pytest test identifier.")
str | None, m.Field(description="Current pytest test identifier.")
] = None
cli_shell_command: Annotated[
str | None,
Field(description="Current shell command propagated by the runtime."),
m.Field(description="Current shell command propagated by the runtime."),
] = None


Expand Down
Loading