Skip to content

fix(trash): strip only trailing trash suffix in to_origin_dir_path - #2341

Open
Siraryansingh wants to merge 16 commits into
oscal-compass:developfrom
Siraryansingh:fix/trash-to-origin-dir-path-bk
Open

fix(trash): strip only trailing trash suffix in to_origin_dir_path#2341
Siraryansingh wants to merge 16 commits into
oscal-compass:developfrom
Siraryansingh:fix/trash-to-origin-dir-path-bk

Conversation

@Siraryansingh

Copy link
Copy Markdown

What this PR does / why we need it:

Fixes to_origin_dir_path() and to_origin_file_path() in trestle/common/trash.py so they strip only the trailing __bk and .bk suffix instead of splitting on all occurrences of __bk or .bk inside directory and file names.

Previously, directory names containing __bk (e.g. alpha__bkbeta) were incorrectly truncated to alpha during recovery.

Which issue(s) this PR fixes:

Fixes #2210

How was this tested?:

  • Added test_to_origin_dir_path_with_bk_in_name unit test in tests/trestle/utils/trash_test.py
  • Ran all 17 trash unit tests via pytest (17 passed in 0.04s)
  • Linted with ruff (all checks passed)

Acuspeedster and others added 7 commits August 22, 2026 05:39
…o all CI doc steps (oscal-compass#2132)

Signed-off-by: Acuspeedster <arnavrajsingh@gmail.com>
Co-authored-by: Lou DeGenaro <lou.degenaro@gmail.com>
…#2336)

Bumps the codeql-action group with 2 updates: [github/codeql-action/init](https://github.com/github/codeql-action) and [github/codeql-action/analyze](https://github.com/github/codeql-action).


Updates `github/codeql-action/init` from 4.37.7 to 4.37.8
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@ff2f1c6...db488dd)

Updates `github/codeql-action/analyze` from 4.37.7 to 4.37.8
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@ff2f1c6...db488dd)

---
updated-dependencies:
- dependency-name: github/codeql-action/init
  dependency-version: 4.37.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: codeql-action
- dependency-name: github/codeql-action/analyze
  dependency-version: 4.37.8
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: codeql-action
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: degenaro <lou.degenaro@gmail.com>
…ss#2337)

* fix: agentic-agile-authoring to Learn About The Projects

Signed-off-by: degenaro <lou.degenaro@gmail.com>

* fix: revised text

Signed-off-by: degenaro <lou.degenaro@gmail.com>

---------

Signed-off-by: degenaro <lou.degenaro@gmail.com>
by_alias/by_name/extra were added to BaseModel.model_validate in pydantic
2.11.0; the lower bound now matches the version that introduced them.

Signed-off-by: 谢翊凡 <xyf5432@users.noreply.github.com>
Co-authored-by: 谢翊凡 <xyf5432@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Lou DeGenaro <lou.degenaro@gmail.com>
Signed-off-by: Acuspeedster <arnavrajsingh@gmail.com>
Co-authored-by: Lou DeGenaro <lou.degenaro@gmail.com>
Fix to_origin_dir_path and to_origin_file_path so they strip only the trailing __bk and .bk suffix rather than splitting on all occurrences of __bk or .bk inside directory and file names.

Fixes oscal-compass#2210

Signed-off-by: Aryan Singh <aryansingh.as1012@gmail.com>
@Siraryansingh
Siraryansingh requested a review from a team as a code owner August 29, 2026 12:39
dependabot Bot and others added 5 commits August 31, 2026 05:37
…l-compass#2343)

Bumps [python-semantic-release/python-semantic-release](https://github.com/python-semantic-release/python-semantic-release) from 10.6.1 to 10.6.2.
- [Release notes](https://github.com/python-semantic-release/python-semantic-release/releases)
- [Changelog](https://github.com/python-semantic-release/python-semantic-release/blob/master/CHANGELOG.rst)
- [Commits](python-semantic-release/python-semantic-release@39dd205...9a026e9)

---
updated-dependencies:
- dependency-name: python-semantic-release/python-semantic-release
  dependency-version: 10.6.2
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…#2342)

Bumps the codeql-action group with 2 updates: [github/codeql-action/init](https://github.com/github/codeql-action) and [github/codeql-action/analyze](https://github.com/github/codeql-action).


Updates `github/codeql-action/init` from 4.37.8 to 4.37.9
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@db488dd...cdf488f)

Updates `github/codeql-action/analyze` from 4.37.8 to 4.37.9
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@db488dd...cdf488f)

---
updated-dependencies:
- dependency-name: github/codeql-action/init
  dependency-version: 4.37.9
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: codeql-action
- dependency-name: github/codeql-action/analyze
  dependency-version: 4.37.9
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: codeql-action
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Lou DeGenaro <lou.degenaro@gmail.com>
…scal-compass#2195)

* test(validate): enable component-definition port validation coverage

Signed-off-by: Acuspeedster <arnavrajsingh@gmail.com>

* fix: validation error

Signed-off-by: degenaro <lou.degenaro@gmail.com>

---------

Signed-off-by: Acuspeedster <arnavrajsingh@gmail.com>
Signed-off-by: degenaro <lou.degenaro@gmail.com>
Co-authored-by: Lou DeGenaro <lou.degenaro@gmail.com>
@degenaro

Copy link
Copy Markdown
Collaborator

@Siraryansingh Thx for this PR. Are you on our slack channel #oscal-compass-trestle-agileauthoring-c2p? To join you need a LF ID, you can get one here https://identity.linuxfoundation.org/.

Below is Bob's review. Please fix the DCO and missing test issues, thx!

=====

PR #2341 Review — fix(trash): strip only trailing trash suffix in to_origin_dir_path

Author: Aryan Singh (@Siraryansingh)
Base: develop
Files changed: trestle/common/trash.py (+7 / -4), tests/trestle/utils/trash_test.py (+10 / 0)
Issue fixed: #2210


🚫 Blocker — DCO not correctly signed

The commit 6e1a56a contains:

Signed-off-by: Aryan Singh <aryansingh.as1012@gmail.com>

However, the commit was authored under the GitHub identity siraryansingh2005@gmail.com. The DCO requires that the email in the Signed-off-by trailer exactly matches the email associated with the commit author identity. Because these two addresses differ, the DCO check fails.

Resolution: The author must amend the commit so that the Signed-off-by email matches the author email on record, then force-push:

git commit --amend --signoff
# verify the Signed-off-by email matches your git user.email, then:
git push --force-with-lease

Summary

This PR fixes a real bug in trestle/common/trash.py: both to_origin_dir_path() and to_origin_file_path() used str.split() to remove the __bk / .bk trash suffixes. That approach silently truncates any path component or filename that contains those strings in the middle — for example, a directory named alpha__bkbeta would be recovered as alpha.


Fix Quality ✅ Correct

The change is minimal and precise.

Before (same pattern in both functions):

parts = item.split(TRESTLE_TRASH_DIR_EXT)
origin_path_parts.append(parts[0])

str.split() splits on every occurrence, so alpha__bkbeta__bk['alpha', 'beta', '']'alpha'.

After:

if item.endswith(TRESTLE_TRASH_DIR_EXT):
    item = item[:-len(TRESTLE_TRASH_DIR_EXT)]
origin_path_parts.append(item)

This correctly strips only the trailing suffix. alpha__bkbeta__bkalpha__bkbeta. ✅

The fix is applied symmetrically to both to_origin_dir_path() (directory parts) and to_origin_file_path() (filename).


Test Coverage ✅ Adequate — one minor gap

  • test_to_origin_dir_path_with_bk_in_name correctly exercises to_origin_dir_path() with a directory whose name contains __bk in the middle.
  • All 17 trash unit tests pass locally.

Minor gap (non-blocking): There is no analogous regression test for to_origin_file_path() with a filename that contains .bk in the middle (e.g. report.bk2.md). A suggested addition:

def test_to_origin_file_path_with_bk_in_name(tmp_path: pathlib.Path) -> None:
    """Test to_origin_file_path handles file names containing .bk."""
    test_utils.ensure_trestle_config_dir(tmp_path)
    (tmp_path / trash.TRESTLE_TRASH_DIR).mkdir(exist_ok=True, parents=True)
    bk_file = tmp_path / 'report.bk2.md'
    trash_file_path = trash.to_trash_file_path(bk_file)
    origin_file_path = trash.to_origin_file_path(trash_file_path)
    assert bk_file.resolve() == origin_file_path.resolve()

Round-trip Correctness ✅

to_trash_dir_path() appends the suffix to the full relative path string exactly once. The fixed to_origin_dir_path() now strips it from the end exactly once. The forward/reverse round-trip is mathematically consistent.


Code Style ✅

item[:-len(TRESTLE_TRASH_DIR_EXT)] is idiomatic Python. An equally readable alternative is item.removesuffix(TRESTLE_TRASH_DIR_EXT) (Python 3.9+), but the current form is acceptable.


Overall Verdict

Concern Status
DCO signature mismatch 🚫 Blocker
Logic correctness ✅ Correct
Test for directory case ✅ Present
Test for file case with .bk in name ⚠️ Missing (non-blocking suggestion)
Code style ✅ Fine

The fix itself is correct and ready to merge once the DCO issue is resolved.

@degenaro

Copy link
Copy Markdown
Collaborator

Also, some tips. Run locally:

make develop
make code-format
make code-lint
make mdformat
make test

@degenaro

Copy link
Copy Markdown
Collaborator

Some additional recommendations:

degenaro and others added 4 commits September 1, 2026 13:38
* fix: improve tests

Signed-off-by: degenaro <lou.degenaro@gmail.com>

* fix: format

Signed-off-by: degenaro <lou.degenaro@gmail.com>

---------

Signed-off-by: degenaro <lou.degenaro@gmail.com>
…e datetime (oscal-compass#2139)

- jinja/tags.py: replace type(x is str) with isinstance(x, str).
  type(kwargs['format'] is str) always evaluated to <class 'bool'> (truthy),
  making the else branch (default format fallback) unreachable dead code and
  silently passing non-string values to strftime().

- catalog_merger.py: remove 7 list_utils imports that were never used
  (as_dict, as_filtered_list, as_list, delete_item_from_list, deep_set,
  get_item_from_list, none_if_empty). Only set_or_pop is actually called.

- cli.py: replace exit() with sys.exit(). The built-in exit() is intended
  for interactive interpreter use; sys.exit() is correct for application code.
  Add missing 'import sys'.

- cache.py: pass tz=datetime.timezone.utc to both fromtimestamp() and now()
  so the timedelta calculation is timezone-aware and immune to DST transitions.

Signed-off-by: Acuspeedster <arnavrajsingh@gmail.com>
Co-authored-by: Lou DeGenaro <lou.degenaro@gmail.com>
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.

to_origin_dir_path truncates directory names containing __bk

4 participants