Skip to content

fix: cos share all#2050

Merged
edwinjosechittilappilly merged 3 commits into
release-saas-ga-0.6.2from
cos-fix
Jul 8, 2026
Merged

fix: cos share all#2050
edwinjosechittilappilly merged 3 commits into
release-saas-ga-0.6.2from
cos-fix

Conversation

@edwinjosechittilappilly

Copy link
Copy Markdown
Collaborator

This pull request introduces several improvements and bug fixes related to the IBM Cloud Object Storage (COS) connector, particularly focusing on local development enablement, correct handling of the "shared" ownership flag during connector sync, and robust unit testing for these behaviors. The changes ensure that the IBM COS connector can be safely enabled for local development, that ownership fields are properly updated for unchanged files when toggling the "shared" setting, and that these behaviors are thoroughly tested.

IBM COS connector: Local development enablement and gating

  • Added new environment variables (OPENRAG_DEV_IBM_COS, NEXT_PUBLIC_DEV_IBM_COS) and corresponding logic to allow the IBM COS connector to be enabled for local development and testing (e.g., with MinIO), bypassing the IBM_AUTH_ENABLED gate, but never in production. This includes both backend (is_dev_ibm_cos_enabled) and frontend (DEV_IBM_COS) support, and updates to visibility logic in the UI. (.env.example [1] enhancements/connectors/ibm_cos/connector.py [2] [3] src/config/settings.py [4] frontend/lib/brand.ts [5] [6]

Connector sync: Correct propagation and handling of the "shared" flag

  • Ensured that the shared flag is correctly forwarded to all relevant sync batches (new and changed files) when syncing connectors via the bucket_filter path, fixing a bug where the "Make documents available to all users" toggle had no effect for IBM COS syncs. (src/api/connectors.py [1] [2]

Ownership reconciliation: Fix for unchanged/duplicate files

  • Implemented _reconcile_shared_owner in ConnectorFileProcessor to update ownership fields in already-indexed chunks when the "shared" setting is toggled, even if the file content and name are unchanged. This prevents stale ownership metadata for files skipped as duplicates or unchanged during sync. (src/models/processors.py [1] [2] [3]

Unit tests: Coverage for new gating, shared flag, and reconciliation logic

  • Added unit tests to verify IBM COS connector gating (auth/dev flags), correct propagation of the shared flag in sync, and correct behavior of _reconcile_shared_owner (including error handling and script logic). (tests/unit/connectors/test_ibm_cos_connector.py [1] tests/unit/test_connector_sync_bucket_filter_shared.py [2] tests/unit/test_reconcile_shared_owner.py [3] tests/unit/test_shared_flag.py [4] [5]

These changes collectively improve the reliability, testability, and developer experience of the IBM COS connector, especially around local development and document sharing features.

… dedupe skip

Two related bugs kept "Make documents available to all users" from actually
sharing documents:

1. connector_sync's bucket_filter branch (the path the COS connector UI's
   "Ingest N Buckets" button always hits) never forwarded body.shared to
   either of its two sync_specific_files calls (new files / changed files),
   so newly-ingested files always ended up privately owned regardless of the
   toggle.

2. ConnectorFileProcessor.process_item's duplicate-filename and
   unchanged-hash skip branches returned before resolve_shared_owner_fields()
   ever ran, so re-syncing a bucket whose files were already indexed left
   their ownership exactly as it was on the prior sync.

Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query
scoped to owner==self OR ownerless (same boundary as
delete_document_by_filename), called from both skip paths instead of
silently leaving stale ownership in place.

Also fixes two pre-existing test_shared_flag.py tests that had drifted from
connector_sync's current signature (missing request/session/rbac args).
… enabled

Found during review: is_dev_ibm_cos_enabled() previously defaulted
OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching
is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only
IBM COS connector available in every deployment by default. The existing
tests only monkeypatched the function away, so they wouldn't have caught
this. Add a test asserting the unset-env-var default.
@github-actions github-actions Bot added frontend 🟨 Issues related to the UI/UX backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) tests bug 🔴 Something isn't working. labels Jul 8, 2026
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c2358a54-5669-427d-bc93-f2b521a34fa4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch cos-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added bug 🔴 Something isn't working. and removed bug 🔴 Something isn't working. labels Jul 8, 2026
@ricofurtado

Copy link
Copy Markdown
Collaborator

LGTM

@ricofurtado
ricofurtado self-requested a review July 8, 2026 15:00
@github-actions github-actions Bot added the lgtm label Jul 8, 2026
@edwinjosechittilappilly
edwinjosechittilappilly merged commit a403461 into release-saas-ga-0.6.2 Jul 8, 2026
29 of 37 checks passed
@github-actions
github-actions Bot deleted the cos-fix branch July 8, 2026 15:39
edwinjosechittilappilly added a commit that referenced this pull request Jul 15, 2026
* feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025)

* feature Flag Share all

* style: apply biome auto-fixes [skip ci]

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* make shareall default as true (#2035)

* fix: cos share all (#2050)

* fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip

Two related bugs kept "Make documents available to all users" from actually
sharing documents:

1. connector_sync's bucket_filter branch (the path the COS connector UI's
   "Ingest N Buckets" button always hits) never forwarded body.shared to
   either of its two sync_specific_files calls (new files / changed files),
   so newly-ingested files always ended up privately owned regardless of the
   toggle.

2. ConnectorFileProcessor.process_item's duplicate-filename and
   unchanged-hash skip branches returned before resolve_shared_owner_fields()
   ever ran, so re-syncing a bucket whose files were already indexed left
   their ownership exactly as it was on the prior sync.

Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query
scoped to owner==self OR ownerless (same boundary as
delete_document_by_filename), called from both skip paths instead of
silently leaving stale ownership in place.

Also fixes two pre-existing test_shared_flag.py tests that had drifted from
connector_sync's current signature (missing request/session/rbac args).

* test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled

Found during review: is_dev_ibm_cos_enabled() previously defaulted
OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching
is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only
IBM COS connector available in every deployment by default. The existing
tests only monkeypatched the function away, so they wouldn't have caught
this. Add a test asserting the unset-env-var default.

* style: ruff autofix (auto)

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
edwinjosechittilappilly added a commit that referenced this pull request Jul 15, 2026
…#2095)

* feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025)

* feature Flag Share all

* style: apply biome auto-fixes [skip ci]

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* make shareall default as true (#2035)

* fix: cos share all (#2050)

* fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip

Two related bugs kept "Make documents available to all users" from actually
sharing documents:

1. connector_sync's bucket_filter branch (the path the COS connector UI's
   "Ingest N Buckets" button always hits) never forwarded body.shared to
   either of its two sync_specific_files calls (new files / changed files),
   so newly-ingested files always ended up privately owned regardless of the
   toggle.

2. ConnectorFileProcessor.process_item's duplicate-filename and
   unchanged-hash skip branches returned before resolve_shared_owner_fields()
   ever ran, so re-syncing a bucket whose files were already indexed left
   their ownership exactly as it was on the prior sync.

Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query
scoped to owner==self OR ownerless (same boundary as
delete_document_by_filename), called from both skip paths instead of
silently leaving stale ownership in place.

Also fixes two pre-existing test_shared_flag.py tests that had drifted from
connector_sync's current signature (missing request/session/rbac args).

* test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled

Found during review: is_dev_ibm_cos_enabled() previously defaulted
OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching
is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only
IBM COS connector available in every deployment by default. The existing
tests only monkeypatched the function away, so they wouldn't have caught
this. Add a test asserting the unset-env-var default.

* style: ruff autofix (auto)

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* missing ingest settings for individual files (#2066)

Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly.

Changes (frontend-only, no backend changes needed — it already threads shared uniformly):
- frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path.
- frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
edwinjosechittilappilly added a commit that referenced this pull request Jul 16, 2026
…ors (backport of #2077) (#2096)

* feat: add dedicated OPENRAG_SHOW_SHARED_UPLOAD_TOGGLE flag for COS "share all" toggle (#2025)

* feature Flag Share all

* style: apply biome auto-fixes [skip ci]

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

* make shareall default as true (#2035)

* fix: cos share all (#2050)

* fix: COS shared-toggle owner not propagated on bucket sync + stale on dedupe skip

Two related bugs kept "Make documents available to all users" from actually
sharing documents:

1. connector_sync's bucket_filter branch (the path the COS connector UI's
   "Ingest N Buckets" button always hits) never forwarded body.shared to
   either of its two sync_specific_files calls (new files / changed files),
   so newly-ingested files always ended up privately owned regardless of the
   toggle.

2. ConnectorFileProcessor.process_item's duplicate-filename and
   unchanged-hash skip branches returned before resolve_shared_owner_fields()
   ever ran, so re-syncing a bucket whose files were already indexed left
   their ownership exactly as it was on the prior sync.

Fix (2) via _reconcile_shared_owner(): a metadata-only update_by_query
scoped to owner==self OR ownerless (same boundary as
delete_document_by_filename), called from both skip paths instead of
silently leaving stale ownership in place.

Also fixes two pre-existing test_shared_flag.py tests that had drifted from
connector_sync's current signature (missing request/session/rbac args).

* test: guard is_dev_ibm_cos_enabled's default against re-regressing to enabled

Found during review: is_dev_ibm_cos_enabled() previously defaulted
OPENRAG_DEV_IBM_COS to "true" when unset (now fixed to "false", matching
is_dev_azure_blob_enabled's pattern), which made the Enterprise/SaaS-only
IBM COS connector available in every deployment by default. The existing
tests only monkeypatched the function away, so they wouldn't have caught
this. Add a test asserting the unset-env-var default.

* style: ruff autofix (auto)

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

* missing ingest settings for individual files (#2066)

Root cause: FileBrowserDialog (the "Browse Files" individual-file picker) never read the "Make documents available to all users" toggle or ingest settings — it built its sync request with only selected_files, so shared silently defaulted to false on the backend. The whole-bucket ("folder") ingest path in SharedBucketView already did this correctly.

Changes (frontend-only, no backend changes needed — it already threads shared uniformly):
- frontend/components/file-browser-dialog.tsx: added showShared/ingestSettings props, and now includes settings and shared in the sync request body, mirroring the bucket-ingest path.
- frontend/components/connectors/shared-bucket-view.tsx: passes its existing showSharedToggle/ingestSettings state down into FileBrowserDialog.

* fix: introduces a user-facing duplicate handling workflow for bucket-based connectors  (#2077)

* fix japanese file names

Root cause: COS/Azure/S3 connectors build document.id as the raw, non-ASCII object key ("bucket::報告書.pdf"), and that value was reused verbatim as document_id in the default Langflow ingestion path — the only ingestion path that puts non-ASCII text into ASCII-only HTTP headers and an unbounded-length OpenSearch identifier. Google Drive (opaque ASCII ID) and manual upload (content hash) were structurally immune; COS/Azure/S3 were the only paths exposed.

Fix applied (matches the already-proven split used by the non-Langflow ingestion path):
- document_index_writer.py — DocumentIndexContext gained a connector_file_id field, now written onto every indexed chunk when present.
- langflow_file_service.py — _resolve_document_id now hashes any supplied connector_file_id into a stable ASCII document_id, instead of using it verbatim; connector_file_id threads through run_ingestion_flow → _configure_ingest_callback → the JWT context.
- processors.py — the Langflow-path connector branch now passes connector_file_id=document.id (not document_id=document.id), and the post-ingest existence check / metadata sync now match on connector_file_id.
- connectors/service.py — stale comments corrected (dedupe/ACL-sync already matched both fields, no logic change needed there).
- Added 13 new unit tests (test_resolve_document_id_connector_file_id.py, test_document_index_writer_connector_file_id.py, test_bucket_connector_unicode_file_ids.py) — confirmed they fail on the old code and pass on the fix.
- Ran the full unit suite: same 14 pre-existing failures before and after (unrelated to this change — auth/settings/encryption tests), no new regressions.

* style: ruff autofix (auto)

* fix connector File name issue

* ingest fix

* Update processors.py

* review fix

* fix connector dialog on rewrite

* overwrite dialog box

* Update connectors.py

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend 🔷 Issues related to backend services (OpenSearch, Langflow, APIs) bug 🔴 Something isn't working. frontend 🟨 Issues related to the UI/UX lgtm tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants